fix(lark): scope group recent_context to the Lark topic (MUL-5224) (#5882)

Feishu 话题 sessions are isolated per topic (#5061), but the group
recent_context prefetch read the whole chat: a @-mention inside topic B
could pull topic A's messages into B's prompt and persist them into B's
turn (#5835). Fix the leak at the fetch:

- ListChatMessages sends container_id_type=thread&container_id=<thread_id>
  when a topic id is present; the thread container rejects end_time, so the
  window is anchored to the trigger time client-side instead.
- Parse thread_id off the REST item and fail-close on it: a topic fetch
  keeps only exact thread_id matches, dropping any missing/mismatched item
  so a sibling topic can never leak even if the API returns one.
- A topic fetch failure degrades to the readable note and NEVER falls back
  to a chat-wide fetch (that would re-open the leak); a very busy topic may
  get empty context, which is the safe trade.
- Exclude the Bot's own interactive-card replies from the window (they
  flatten to a zero-signal [interactive card] placeholder).
- Non-topic group @-mentions keep the chat-level, end_time-anchored path
  unchanged.

Closes #5835

Co-authored-by: Bohan-J <bohan@devv.ai>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Bohan Jiang
2026-07-24 15:38:13 +08:00
committed by GitHub
parent 3f2e1c68d7
commit 07a0a4272d
5 changed files with 383 additions and 30 deletions

View File

@@ -83,10 +83,16 @@ type APIClient interface {
GetMessage(ctx context.Context, creds InstallationCredentials, messageID string) ([]LarkMessage, error)
// ListChatMessages fetches the most recent messages in a single chat
// via GET /open-apis/im/v1/messages?container_id_type=chat. It powers
// the group-context prefetch: when a user @-mentions the Bot in a busy
// group, the enricher pulls a bounded window of surrounding messages
// so the agent sees the conversation, not just the one @-ed line.
// via GET /open-apis/im/v1/messages. It powers the group-context
// prefetch: when a user @-mentions the Bot in a busy group, the
// enricher pulls a bounded window of surrounding messages so the agent
// sees the conversation, not just the one @-ed line.
//
// When p.ThreadID is set the request is scoped to a single Lark topic
// (话题) via container_id_type=thread, so a @-mention inside a topic
// only ever sees that topic's messages — sibling topics in the same
// chat share one chat_id but must stay isolated (#5835). Empty ThreadID
// keeps the chat-level container_id_type=chat window.
//
// Results come back newest-first (sort_type=ByCreateTimeDesc), capped
// at p.PageSize (Lark hard-caps a page at 50); the caller orders and
@@ -120,11 +126,19 @@ type APIClient interface {
// ListMessagesParams selects a bounded, recent window of messages in a
// single Lark chat for the group-context prefetch. Only the fields the
// enricher needs today are exposed (ChatID, PageSize, EndTime);
// enricher needs today are exposed (ChatID, ThreadID, PageSize, EndTime);
// start_time and page_token are intentionally omitted until a caller
// needs them.
type ListMessagesParams struct {
ChatID ChatID
// ThreadID, when non-empty, scopes the list to a single Lark topic
// (话题): the client sends container_id_type=thread with the thread id
// as container_id instead of the chat container. This keeps a
// @-mention inside a topic from ever seeing sibling topics' messages
// (#5835). Lark's thread container does NOT accept end_time, so EndTime
// is ignored on this path — the caller anchors the window client-side.
// Empty keeps the chat-level container.
ThreadID string
// PageSize is how many of the most-recent messages to fetch. The
// client clamps it into Lark's valid 1..50 range.
PageSize int
@@ -132,7 +146,8 @@ type ListMessagesParams struct {
// this Unix timestamp in SECONDS (Lark's end_time is second-, not
// millisecond-, granularity). The enricher sets it to the trigger
// message's time so the prefetch is anchored to the @-mention moment
// rather than whatever is newest by the time the fetch runs.
// rather than whatever is newest by the time the fetch runs. Ignored
// when ThreadID is set (the thread container rejects end_time).
EndTime int64
}
@@ -149,6 +164,7 @@ type LarkMessage struct {
CreateTime string // epoch milliseconds, as Lark returns it (a string)
ParentID string
RootID string
ThreadID string // Lark topic (话题) id; empty for messages outside a thread
UpperMessageID string // the merge_forward parent a child hangs under
Deleted bool
Mentions []LarkMessageMention

View File

@@ -596,15 +596,18 @@ func (c *httpAPIClient) GetMessage(ctx context.Context, creds InstallationCreden
// silently gets the max rather than a 400 from Lark.
const larkListMessagesMaxPageSize = 50
// ListChatMessages retrieves a bounded, recent window of messages in one
// chat via GET /open-apis/im/v1/messages?container_id_type=chat. Where
// GetMessage fetches a single message by id, this lists a conversation;
// it backs the enricher's group-context prefetch. We pass
// sort_type=ByCreateTimeDesc so the newest messages come first and a
// small page_size captures "the last N" without paginating, keeping the
// inbound ACK path's fan-out to a single round-trip. user_id_type=open_id
// matches the identifiers the rest of the package keys on; body.content
// is forwarded verbatim for the enricher's flattener to interpret.
// ListChatMessages retrieves a bounded, recent window of messages via
// GET /open-apis/im/v1/messages. Where GetMessage fetches a single message
// by id, this lists a conversation; it backs the enricher's group-context
// prefetch. The container is chat (container_id_type=chat) by default, or a
// single Lark topic (container_id_type=thread) when p.ThreadID is set — the
// thread container keeps a topic @-mention from seeing sibling topics that
// share the chat_id (#5835). We pass sort_type=ByCreateTimeDesc so the
// newest messages come first and a small page_size captures "the last N"
// without paginating, keeping the inbound ACK path's fan-out to a single
// round-trip. user_id_type=open_id matches the identifiers the rest of the
// package keys on; body.content is forwarded verbatim for the enricher's
// flattener to interpret.
func (c *httpAPIClient) ListChatMessages(ctx context.Context, creds InstallationCredentials, p ListMessagesParams) ([]LarkMessage, error) {
if p.ChatID == "" {
return nil, errors.New("lark http client: missing chat_id")
@@ -620,14 +623,24 @@ func (c *httpAPIClient) ListChatMessages(ctx context.Context, creds Installation
return nil, err
}
q := url.Values{}
q.Set("container_id_type", "chat")
q.Set("container_id", string(p.ChatID))
if p.ThreadID != "" {
// Topic-scoped window: only this 话题's messages, so a @-mention
// inside a topic never pulls sibling topics that share the chat_id
// (#5835). The thread container rejects end_time, so it is omitted
// here; the caller anchors the window to the trigger time
// client-side instead.
q.Set("container_id_type", "thread")
q.Set("container_id", p.ThreadID)
} else {
q.Set("container_id_type", "chat")
q.Set("container_id", string(p.ChatID))
if p.EndTime > 0 {
q.Set("end_time", strconv.FormatInt(p.EndTime, 10))
}
}
q.Set("sort_type", "ByCreateTimeDesc")
q.Set("page_size", strconv.Itoa(size))
q.Set("user_id_type", "open_id")
if p.EndTime > 0 {
q.Set("end_time", strconv.FormatInt(p.EndTime, 10))
}
path := "/open-apis/im/v1/messages?" + q.Encode()
var resp struct {
@@ -790,6 +803,7 @@ type larkRESTMessageItem struct {
MessageID string `json:"message_id"`
RootID string `json:"root_id"`
ParentID string `json:"parent_id"`
ThreadID string `json:"thread_id"`
UpperMessageID string `json:"upper_message_id"`
MsgType string `json:"msg_type"`
CreateTime string `json:"create_time"`
@@ -819,6 +833,7 @@ func (it larkRESTMessageItem) normalize() LarkMessage {
CreateTime: it.CreateTime,
ParentID: it.ParentID,
RootID: it.RootID,
ThreadID: it.ThreadID,
UpperMessageID: it.UpperMessageID,
Deleted: it.Deleted,
}

View File

@@ -104,6 +104,64 @@ func TestHTTPClient_ListChatMessagesClampsPageSize(t *testing.T) {
}
}
// TestHTTPClient_ListChatMessagesThreadScoped pins the #5835 topic path:
// when ThreadID is set the request switches to container_id_type=thread
// with the topic id as container_id, and end_time is omitted even if the
// caller passes one (the thread container rejects it). The returned items'
// thread_id is normalized onto LarkMessage so the enricher can fail-close
// on it.
func TestHTTPClient_ListChatMessagesThreadScoped(t *testing.T) {
fake := newLarkFake(t)
fake.stubToken("tok", 7200)
fake.mux.HandleFunc("/open-apis/im/v1/messages", func(w http.ResponseWriter, r *http.Request) {
q := r.URL.Query()
if q.Get("container_id_type") != "thread" {
t.Errorf("container_id_type = %q, want thread", q.Get("container_id_type"))
}
if q.Get("container_id") != "th_topic" {
t.Errorf("container_id = %q, want th_topic", q.Get("container_id"))
}
// The thread container rejects end_time; it must be dropped even
// though the caller passed EndTime.
if q.Has("end_time") {
t.Errorf("end_time must be absent on the thread container, got %q", q.Get("end_time"))
}
if q.Get("sort_type") != "ByCreateTimeDesc" {
t.Errorf("sort_type = %q", q.Get("sort_type"))
}
if q.Get("page_size") != "10" {
t.Errorf("page_size = %q", q.Get("page_size"))
}
writeJSON(w, map[string]any{
"code": 0, "msg": "ok",
"data": map[string]any{
"items": []any{
map[string]any{
"message_id": "om_1",
"msg_type": "text",
"create_time": "1000",
"thread_id": "th_topic",
"sender": map[string]any{"id": "ou_a", "id_type": "open_id", "sender_type": "user"},
"body": map[string]any{"content": `{"text":"hi"}`},
},
},
},
})
})
c := newTestClient(fake, time.Now)
items, err := c.ListChatMessages(context.Background(), testCreds(), ListMessagesParams{ChatID: "oc_chat", ThreadID: "th_topic", PageSize: 10, EndTime: 1700000000})
if err != nil {
t.Fatalf("ListChatMessages: %v", err)
}
if len(items) != 1 {
t.Fatalf("items = %d, want 1", len(items))
}
if items[0].ThreadID != "th_topic" {
t.Errorf("normalized ThreadID = %q, want th_topic", items[0].ThreadID)
}
}
// TestHTTPClient_ListChatMessagesMissingChatID fails fast (no token, no
// network call) when the chat id is empty.
func TestHTTPClient_ListChatMessagesMissingChatID(t *testing.T) {

View File

@@ -123,7 +123,9 @@ func NewInboundEnricher(client APIClient, cfg InboundEnricherConfig) Enricher {
// addressed to the Bot, and only when RecentContextSize > 0 — it answers
// MUL-3084 (the Bot saw only the single @-ed line, never the surrounding
// conversation). It is the one fetch here NOT triggered by something the
// user explicitly attached.
// user explicitly attached. When the @-mention arrives inside a Lark topic
// (话题) the window is scoped to that topic, so a topic's context never
// includes a sibling topic's messages (#5835 — see fetchRecentItems).
//
// In group chats, every speaker across ALL blocks (recent + quoted +
// forwarded) and the sender who @-mentioned the Bot are resolved to real
@@ -290,11 +292,19 @@ func (e *inboundEnricher) resolveNames(ctx context.Context, creds InstallationCr
// fetchRecentItems pulls the recent group window and returns the
// messages to render — the trigger message itself and the directly-quoted
// parent (which gets its own <quoted_message> block) filtered out, sorted
// oldest-first. The window is anchored to the trigger message's time so
// it captures the conversation up to the @-mention rather than whatever
// is newest by the time this fetch runs. A fetch failure is returned to
// the caller (which renders a safe, readable degradation note); it never
// blocks ingestion.
// oldest-first. A fetch failure is returned to the caller (which renders a
// safe, readable degradation note); it never blocks ingestion.
//
// When the trigger arrived inside a Lark topic (msg.ThreadID != ""), the
// window is scoped to that topic (container_id_type=thread) so sibling
// topics that share the chat_id can't leak into this topic's context or
// its persisted turn (#5835). Because the thread container rejects
// end_time, the topic path anchors to the trigger time CLIENT-side; it
// also fail-closes on thread_id — any returned item whose thread_id is
// missing or does not match is dropped rather than trusted. A topic fetch
// failure degrades exactly like the chat path and NEVER falls back to a
// chat-wide fetch (that would re-open the leak). Outside a topic the chat
// path is unchanged: anchored to the trigger time via end_time.
func (e *inboundEnricher) fetchRecentItems(ctx context.Context, creds InstallationCredentials, msg InboundMessage) ([]LarkMessage, error) {
if msg.ChatID == "" {
classified := classifyRecentContextFetchError(errRecentContextChannelUnbound)
@@ -302,13 +312,21 @@ func (e *inboundEnricher) fetchRecentItems(ctx context.Context, creds Installati
return nil, errRecentContextChannelUnbound
}
// Lark sends create_time as epoch millis; a missing/unparseable time
// yields 0. The chat path converts it to seconds for end_time; the
// thread path uses the raw millis for the client-side anchor below.
triggerMillis := parseLarkMillis(msg.CreateTime)
params := ListMessagesParams{
ChatID: msg.ChatID,
PageSize: e.recentContextSize,
// Lark sends create_time as epoch millis; end_time wants seconds. A
// missing/unparseable time yields 0, which the client treats as
// "no end_time" (newest N).
EndTime: parseLarkMillis(msg.CreateTime) / 1000,
}
if msg.ThreadID != "" {
// Topic-scoped fetch: no end_time (the thread container rejects it);
// the window is anchored client-side below.
params.ThreadID = msg.ThreadID
} else {
// 0 tells the client "no end_time" (newest N).
params.EndTime = triggerMillis / 1000
}
var items []LarkMessage
var err error
@@ -353,11 +371,33 @@ func (e *inboundEnricher) fetchRecentItems(ctx context.Context, creds Installati
if msg.ParentID != "" {
exclude[msg.ParentID] = true
}
inThread := msg.ThreadID != ""
kept := make([]LarkMessage, 0, len(items))
for _, it := range items {
if exclude[it.MessageID] {
continue
}
// The Bot's markdown replies are sent as schema-2.0 interactive
// cards, which flatten to a zero-signal "[interactive card]"
// placeholder — drop them rather than render noise (#5835).
if it.SenderType == "app" && it.MessageType == "interactive" {
continue
}
if inThread {
// Fail-closed topic isolation: the thread container should only
// return this topic's messages, but if Lark ever returns an item
// with a missing or mismatched thread_id, drop it rather than
// risk leaking a sibling topic's content into this topic.
if it.ThreadID != msg.ThreadID {
continue
}
// The thread container ignores end_time, so anchor client-side:
// drop anything created strictly after the @-mention moment. A
// zero trigger time (unparseable) disables the anchor.
if triggerMillis > 0 && parseLarkMillis(it.CreateTime) > triggerMillis {
continue
}
}
kept = append(kept, it)
}

View File

@@ -25,6 +25,28 @@ func groupCfg() InboundEnricherConfig {
return InboundEnricherConfig{RecentContextSize: DefaultRecentContextSize}
}
// threadTextMsg is textMsg tagged with a Lark topic (话题) id, so tests
// can seed a chat whose returned window interleaves several topics.
func threadTextMsg(id, sender, text, createTime, threadID string) LarkMessage {
m := textMsg(id, sender, text, createTime)
m.ThreadID = threadID
return m
}
// cardMsg builds a Bot-sent interactive card (sender_type "app",
// msg_type "interactive") — the shape the Bot's markdown replies take,
// which flattens to a zero-signal "[interactive card]" placeholder.
func cardMsg(id, createTime string) LarkMessage {
return LarkMessage{
MessageID: id,
MessageType: "interactive",
Content: `{"type":"template"}`,
SenderID: "cli_bot",
SenderType: "app",
CreateTime: createTime,
}
}
func assertNoRecentContextFetchPlaceholder(t *testing.T, body string) {
t.Helper()
if strings.Contains(body, `<recent_context type="error">`) ||
@@ -85,6 +107,208 @@ func TestEnrichRecentContextGroupMention(t *testing.T) {
if got := fake.listParams[0].EndTime; got != 3 {
t.Errorf("end_time = %d, want 3 (3000ms -> 3s)", got)
}
// A non-topic group @-mention uses the chat container, never a thread
// scope — the #5835 topic path must not touch normal-group behavior.
if got := fake.listParams[0].ThreadID; got != "" {
t.Errorf("chat-level fetch must not set ThreadID, got %q", got)
}
}
// TestEnrichRecentContextTopicExcludesOtherTopics is the #5835 core: an
// @-mention inside a Lark topic (话题) must see ONLY that topic's messages,
// never a sibling topic that shares the chat_id. The fetch is topic-scoped
// (ThreadID set, no end_time), and even though the fake returns interleaved
// sibling-topic items the enricher's fail-closed thread_id filter drops
// them, so no sibling content can leak into this topic's context (and,
// downstream, its persisted turn).
func TestEnrichRecentContextTopicExcludesOtherTopics(t *testing.T) {
t.Parallel()
fake := newEnricherFake()
fake.byChat["oc_g"] = []LarkMessage{
threadTextMsg("om_trigger", "ou_user", "总结一下", "3000", "th_a"),
threadTextMsg("om_b2", "ou_carol", "话题B的机密", "2500", "th_b"),
threadTextMsg("om_a2", "ou_bob", "话题A第二条", "2000", "th_a"),
threadTextMsg("om_b1", "ou_dave", "话题B第一条", "1500", "th_b"),
threadTextMsg("om_a1", "ou_alice", "话题A第一条", "1000", "th_a"),
}
in := InboundMessage{
MessageType: "text",
MessageID: "om_trigger",
ChatID: "oc_g",
ChatType: ChatTypeGroup,
AddressedToBot: true,
Body: "总结一下",
CreateTime: "3000",
ThreadID: "th_a",
}
out := enrich(t, fake, in, groupCfg())
want := `<recent_context count="2">
[User 1]: 话题A第一条
[User 2]: 话题A第二条
</recent_context>
总结一下`
if out.Body != want {
t.Errorf("body\n got = %q\nwant = %q", out.Body, want)
}
// Exactly one fetch, scoped to the trigger's topic, with no end_time
// (the thread container rejects it).
if len(fake.listParams) != 1 {
t.Fatalf("expected one ListChatMessages call, got %d", len(fake.listParams))
}
if got := fake.listParams[0].ThreadID; got != "th_a" {
t.Errorf("list ThreadID = %q, want th_a (thread-scoped fetch)", got)
}
if got := fake.listParams[0].EndTime; got != 0 {
t.Errorf("thread fetch must omit end_time, got EndTime=%d", got)
}
}
// TestEnrichRecentContextTopicFailsClosedOnThreadID pins the second line of
// defense: if Lark's thread container ever returns an item whose thread_id
// is missing or does not match the trigger's topic, the enricher drops it
// rather than trust it. Only the exact-match item survives.
func TestEnrichRecentContextTopicFailsClosedOnThreadID(t *testing.T) {
t.Parallel()
fake := newEnricherFake()
fake.byChat["oc_g"] = []LarkMessage{
threadTextMsg("om_trigger", "ou_user", "怎么办", "3000", "th_a"),
threadTextMsg("om_match", "ou_alice", "本话题内容", "2000", "th_a"),
threadTextMsg("om_other", "ou_bob", "别的话题内容", "1800", "th_b"),
threadTextMsg("om_missing", "ou_carol", "缺少话题字段", "1500", ""),
}
in := InboundMessage{
MessageType: "text",
MessageID: "om_trigger",
ChatID: "oc_g",
ChatType: ChatTypeGroup,
AddressedToBot: true,
Body: "怎么办",
CreateTime: "3000",
ThreadID: "th_a",
}
out := enrich(t, fake, in, groupCfg())
want := `<recent_context count="1">
[User 1]: 本话题内容
</recent_context>
怎么办`
if out.Body != want {
t.Errorf("body\n got = %q\nwant = %q", out.Body, want)
}
}
// TestEnrichRecentContextTopicDropsMessagesAfterTrigger covers the
// client-side time anchor the topic path needs because the thread
// container can't be bounded by end_time: an item created strictly after
// the @-mention is dropped, keeping the window "up to the @-mention" like
// the chat path.
func TestEnrichRecentContextTopicDropsMessagesAfterTrigger(t *testing.T) {
t.Parallel()
fake := newEnricherFake()
fake.byChat["oc_g"] = []LarkMessage{
threadTextMsg("om_after", "ou_bob", "触发之后才发的", "4000", "th_a"),
threadTextMsg("om_trigger", "ou_user", "看下上面", "3000", "th_a"),
threadTextMsg("om_before", "ou_alice", "触发之前的", "2000", "th_a"),
}
in := InboundMessage{
MessageType: "text",
MessageID: "om_trigger",
ChatID: "oc_g",
ChatType: ChatTypeGroup,
AddressedToBot: true,
Body: "看下上面",
CreateTime: "3000",
ThreadID: "th_a",
}
out := enrich(t, fake, in, groupCfg())
want := `<recent_context count="1">
[User 1]: 触发之前的
</recent_context>
看下上面`
if out.Body != want {
t.Errorf("body\n got = %q\nwant = %q", out.Body, want)
}
}
// TestEnrichRecentContextTopicFetchErrorNoChatFallback locks the fail-safe:
// when a topic-scoped fetch fails, the enricher degrades to the readable
// note and NEVER retries as a chat-wide fetch — a chat fallback would
// re-open the exact cross-topic leak this change closes.
func TestEnrichRecentContextTopicFetchErrorNoChatFallback(t *testing.T) {
t.Parallel()
fake := newEnricherFake()
fake.errByChat["oc_g"] = errors.New("boom")
in := InboundMessage{
MessageType: "text",
MessageID: "om_trigger",
ChatID: "oc_g",
ChatType: ChatTypeGroup,
AddressedToBot: true,
Body: "在干嘛",
ThreadID: "th_a",
}
out := enrich(t, fake, in, groupCfg())
want := `[Recent Lark context unavailable; continuing with the latest message.]
在干嘛`
if out.Body != want {
t.Errorf("body\n got = %q\nwant = %q", out.Body, want)
}
assertNoRecentContextFetchPlaceholder(t, out.Body)
if len(fake.listParams) != 1 {
t.Fatalf("expected exactly one ListChatMessages call, got %d", len(fake.listParams))
}
// The single fetch stayed thread-scoped — no chat-wide fallback.
if fake.listParams[0].ThreadID != "th_a" {
t.Errorf("the single fetch must stay thread-scoped, got ThreadID=%q", fake.listParams[0].ThreadID)
}
}
// TestEnrichRecentContextExcludesBotInteractiveCards pins the #5835
// acceptance item that the Bot's own interactive-card replies (which
// flatten to a useless "[interactive card]" placeholder) are excluded from
// the recent-context window rather than rendered as noise.
func TestEnrichRecentContextExcludesBotInteractiveCards(t *testing.T) {
t.Parallel()
fake := newEnricherFake()
fake.byChat["oc_g"] = []LarkMessage{
textMsg("om_trigger", "ou_user", "总结一下", "3000"),
cardMsg("om_card", "2500"),
textMsg("om_a", "ou_alice", "我改完了登录页", "1000"),
}
in := InboundMessage{
MessageType: "text",
MessageID: "om_trigger",
ChatID: "oc_g",
ChatType: ChatTypeGroup,
AddressedToBot: true,
Body: "总结一下",
CreateTime: "3000",
}
out := enrich(t, fake, in, groupCfg())
want := `<recent_context count="1">
[User 1]: 我改完了登录页
</recent_context>
总结一下`
if out.Body != want {
t.Errorf("body\n got = %q\nwant = %q", out.Body, want)
}
if strings.Contains(out.Body, "[interactive card]") {
t.Errorf("bot interactive card placeholder leaked into recent_context: %q", out.Body)
}
}
// TestEnrichRecentContextRendersDeletedItems keeps deleted messages