From b42ec8bfaee16beb8ad75abd5bb7d6a15022c6f6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 24 Jul 2026 14:27:47 -0700 Subject: [PATCH] fix(discord): filter pending history by sender (#113407) * fix(discord): filter pending history by sender * fix(discord): keep history provenance type private * chore: leave release notes to release tooling --- .../discord/src/monitor/inbound-context.ts | 4 +- .../monitor/message-handler.context.test.ts | 102 ++++++++++++++++++ .../src/monitor/message-handler.context.ts | 63 +++++++---- .../src/monitor/message-handler.history.ts | 48 +++++++++ .../message-handler.preflight-history.ts | 14 ++- .../monitor/message-handler.preflight.test.ts | 4 + .../src/monitor/message-handler.preflight.ts | 55 ++++------ .../message-handler.preflight.types.ts | 6 +- ...essage-handler.process.room-events.test.ts | 5 + .../monitor/message-handler.test-harness.ts | 3 +- extensions/discord/src/monitor/provider.ts | 2 +- 11 files changed, 245 insertions(+), 61 deletions(-) create mode 100644 extensions/discord/src/monitor/message-handler.history.ts diff --git a/extensions/discord/src/monitor/inbound-context.ts b/extensions/discord/src/monitor/inbound-context.ts index 1caa4d308897..543c27569e9c 100644 --- a/extensions/discord/src/monitor/inbound-context.ts +++ b/extensions/discord/src/monitor/inbound-context.ts @@ -12,7 +12,7 @@ type DiscordSupplementalContextSender = { id?: string; name?: string; tag?: string; - memberRoleIds?: string[]; + memberRoleIds?: readonly string[]; }; export function createDiscordSupplementalContextAccessChecker(params: { @@ -33,7 +33,7 @@ export function createDiscordSupplementalContextAccessChecker(params: { resolveDiscordMemberAllowed({ userAllowList, roleAllowList, - memberRoleIds: sender.memberRoleIds ?? [], + memberRoleIds: [...(sender.memberRoleIds ?? [])], userId: sender.id ?? "", userName: sender.name, userTag: sender.tag, diff --git a/extensions/discord/src/monitor/message-handler.context.test.ts b/extensions/discord/src/monitor/message-handler.context.test.ts index 4c1073685d95..8bac40bd7982 100644 --- a/extensions/discord/src/monitor/message-handler.context.test.ts +++ b/extensions/discord/src/monitor/message-handler.context.test.ts @@ -1,8 +1,26 @@ // Discord tests cover sender bot-status forwarding into the inbound context payload. import { describe, expect, it } from "vitest"; import { buildDiscordMessageProcessContext } from "./message-handler.context.js"; +import type { DiscordHistoryEntry } from "./message-handler.history.js"; import { createBaseDiscordMessageContext } from "./message-handler.test-harness.js"; +function historyEntry(params: { + id: string; + senderId: string; + sender: string; + body: string; +}): DiscordHistoryEntry { + return { + sender: params.sender, + body: params.body, + messageId: params.id, + senderProvenance: Object.freeze({ + id: params.senderId, + memberRoleIds: Object.freeze([]), + }), + }; +} + describe("discord buildDiscordMessageProcessContext sender bot status", () => { it("preserves the native Discord channel id for tool authorization", async () => { const ctx = await createBaseDiscordMessageContext(); @@ -60,6 +78,7 @@ describe("discord buildDiscordMessageProcessContext sender bot status", () => { guildHistories, historyLimit: 10, inboundEventKind: "room_event", + sender: { id: "U1", label: "user", name: "alice", isPluralKit: false }, message: { id: "m-forwarded", channelId: "c1", @@ -89,5 +108,88 @@ describe("discord buildDiscordMessageProcessContext sender bot status", () => { }); expect(guildHistories.get("c1")?.[0]?.body).toBe(forwardedText); + expect(guildHistories.get("c1")?.[0]?.senderProvenance).toEqual({ + id: "U1", + name: "alice", + memberRoleIds: [], + }); + expect(Object.isFrozen(guildHistories.get("c1")?.[0]?.senderProvenance)).toBe(true); + expect(Object.isFrozen(guildHistories.get("c1")?.[0]?.senderProvenance.memberRoleIds)).toBe( + true, + ); + }); + + it("filters pending and inbound history by sender provenance in allowlist mode", async () => { + const guildHistories = new Map([ + [ + "c1", + [ + historyEntry({ id: "allowed", senderId: "111", sender: "Alice", body: "allowed body" }), + historyEntry({ id: "blocked", senderId: "222", sender: "Mallory", body: "blocked body" }), + ], + ], + ]); + const ctx = await createBaseDiscordMessageContext({ + cfg: { channels: { discord: { contextVisibility: "allowlist" } } }, + guildHistories, + historyLimit: 10, + channelConfig: { allowed: true, users: ["111"] }, + }); + + const result = await buildDiscordMessageProcessContext({ ctx, text: "current", mediaList: [] }); + if (!result) { + throw new Error("expected a built Discord message context"); + } + + expect(result.ctxPayload.Body).toContain("allowed body"); + expect(result.ctxPayload.Body).not.toContain("blocked body"); + expect(result.ctxPayload.InboundHistory).toEqual([ + expect.objectContaining({ messageId: "allowed", body: "allowed body" }), + ]); + }); + + it("keeps all pending and inbound history under the default visibility mode", async () => { + const guildHistories = new Map([ + [ + "c1", + [ + historyEntry({ id: "allowed", senderId: "111", sender: "Alice", body: "allowed body" }), + historyEntry({ id: "other", senderId: "222", sender: "Mallory", body: "other body" }), + ], + ], + ]); + const ctx = await createBaseDiscordMessageContext({ + guildHistories, + historyLimit: 10, + channelConfig: { allowed: true, users: ["111"] }, + }); + + const result = await buildDiscordMessageProcessContext({ ctx, text: "current", mediaList: [] }); + if (!result) { + throw new Error("expected a built Discord message context"); + } + + expect(result.ctxPayload.Body).toContain("allowed body"); + expect(result.ctxPayload.Body).toContain("other body"); + expect(result.ctxPayload.InboundHistory).toHaveLength(2); + }); + + it("does not inject stale pending history when history is disabled", async () => { + const guildHistories = new Map([ + ["c1", [historyEntry({ id: "stale", senderId: "111", sender: "Alice", body: "stale body" })]], + ]); + const ctx = await createBaseDiscordMessageContext({ + guildHistories, + historyLimit: 0, + }); + + const result = await buildDiscordMessageProcessContext({ ctx, text: "current", mediaList: [] }); + if (!result) { + throw new Error("expected a built Discord message context"); + } + + expect(result.ctxPayload.Body).toContain("current"); + expect(result.ctxPayload.Body).not.toContain("stale body"); + expect(result.ctxPayload.InboundHistory).toBeUndefined(); }); }); diff --git a/extensions/discord/src/monitor/message-handler.context.ts b/extensions/discord/src/monitor/message-handler.context.ts index 02c6707bfa0f..f1aa30120afe 100644 --- a/extensions/discord/src/monitor/message-handler.context.ts +++ b/extensions/discord/src/monitor/message-handler.context.ts @@ -9,7 +9,11 @@ import { import { resolveChannelContextVisibilityMode } from "openclaw/plugin-sdk/context-visibility-runtime"; import { resolvePinnedMainDmOwnerFromAllowlist } from "openclaw/plugin-sdk/conversation-runtime"; import { isDangerousNameMatchingEnabled } from "openclaw/plugin-sdk/dangerous-name-runtime"; -import { createChannelHistoryWindow } from "openclaw/plugin-sdk/reply-history"; +import { + buildHistoryContextFromEntries, + buildInboundHistoryFromEntries, + createChannelHistoryWindow, +} from "openclaw/plugin-sdk/reply-history"; import { buildAgentSessionKey, resolveThreadSessionKeys } from "openclaw/plugin-sdk/routing"; import { danger, logVerbose, shouldLogVerbose } from "openclaw/plugin-sdk/runtime-env"; import { evaluateSupplementalContextVisibility } from "openclaw/plugin-sdk/security-runtime"; @@ -24,6 +28,11 @@ import { createDiscordSupplementalContextAccessChecker, } from "./inbound-context.js"; import { resolveDiscordMessageStickers } from "./message-forwarded.js"; +import { + createDiscordHistorySenderProvenance, + filterDiscordHistoryEntriesForContext, + type DiscordHistoryEntry, +} from "./message-handler.history.js"; import type { DiscordMessagePreflightContext } from "./message-handler.preflight.js"; import { removeDiscordReplayHistoryEntry } from "./message-handler.retry.js"; import { @@ -156,6 +165,7 @@ export async function buildDiscordMessageProcessContext(params: { sessionKey: route.sessionKey, }); const channelHistory = createChannelHistoryWindow({ historyMap: guildHistories }); + let visibleChannelHistory: DiscordHistoryEntry[] | undefined; let combinedBody = formatInboundEnvelope({ channel: "Discord", from: fromLabel, @@ -172,21 +182,34 @@ export async function buildDiscordMessageProcessContext(params: { !(isGuildMessage && channelConfig?.autoThread && !threadChannel)); if (shouldIncludeChannelHistory) { removeDiscordReplayHistoryEntry(guildHistories, messageChannelId, message.id); - combinedBody = channelHistory.buildPendingContext({ - historyKey: messageChannelId, - limit: historyLimit, - currentMessage: combinedBody, - formatEntry: (entry) => - formatInboundEnvelope({ - channel: "Discord", - from: fromLabel, - timestamp: entry.timestamp, - body: `${entry.body} [id:${entry.messageId ?? "unknown"} channel:${messageChannelId}]`, - chatType: "channel", - senderLabel: entry.sender, - envelope: envelopeOptions, - }), - }); + if (historyLimit > 0) { + const filteredHistory = filterDiscordHistoryEntriesForContext({ + entries: guildHistories.get(messageChannelId) ?? [], + mode: contextVisibilityMode, + isSenderAllowed: isSupplementalContextSenderAllowed, + }); + visibleChannelHistory = filteredHistory.entries; + if (filteredHistory.omitted > 0) { + logVerbose( + `discord: omit ${filteredHistory.omitted} pending history entries (mode=${contextVisibilityMode})`, + ); + } + combinedBody = buildHistoryContextFromEntries({ + entries: visibleChannelHistory, + currentMessage: combinedBody, + formatEntry: (entry) => + formatInboundEnvelope({ + channel: "Discord", + from: fromLabel, + timestamp: entry.timestamp, + body: `${entry.body} [id:${entry.messageId ?? "unknown"} channel:${messageChannelId}]`, + chatType: "channel", + senderLabel: entry.sender, + envelope: envelopeOptions, + }), + excludeLast: false, + }); + } } const replyContext = resolveReplyContext(message, resolveDiscordMessageText); const replySenderAllowed = replyContext @@ -306,8 +329,8 @@ export async function buildDiscordMessageProcessContext(params: { } const lastRouteTo = dmConversationTarget ?? effectiveTo; const inboundHistory = shouldIncludeChannelHistory - ? channelHistory.buildInboundHistory({ - historyKey: messageChannelId, + ? buildInboundHistoryFromEntries({ + entries: visibleChannelHistory ?? [], limit: historyLimit, }) : undefined; @@ -454,6 +477,10 @@ export async function buildDiscordMessageProcessContext(params: { body: historyText, timestamp: resolveTimestampMs(message.timestamp), messageId: message.id, + senderProvenance: createDiscordHistorySenderProvenance({ + sender, + memberRoleIds, + }), }, media: toHistoryMediaEntries(mediaList, { messageId: message.id }), messageId: message.id, diff --git a/extensions/discord/src/monitor/message-handler.history.ts b/extensions/discord/src/monitor/message-handler.history.ts new file mode 100644 index 000000000000..99f2a3bbd781 --- /dev/null +++ b/extensions/discord/src/monitor/message-handler.history.ts @@ -0,0 +1,48 @@ +// Discord plugin module owns sender provenance for its in-memory history window. +import type { ContextVisibilityMode } from "openclaw/plugin-sdk/config-contracts"; +import type { HistoryEntry } from "openclaw/plugin-sdk/reply-history"; +import { filterSupplementalContextItems } from "openclaw/plugin-sdk/security-runtime"; +import type { DiscordSenderIdentity } from "./sender-identity.js"; + +type DiscordHistorySenderProvenance = Readonly<{ + id: string; + name?: string; + tag?: string; + memberRoleIds: readonly string[]; +}>; + +export type DiscordHistoryEntry = HistoryEntry & { + senderProvenance: DiscordHistorySenderProvenance; +}; + +export function createDiscordHistorySenderProvenance(params: { + sender: Pick; + memberRoleIds: readonly string[]; +}): DiscordHistorySenderProvenance { + // Snapshot admission-time identity instead of later re-parsing an ambiguous display label. + // Freezing keeps context filtering tied to the sender facts that produced the history entry. + return Object.freeze({ + id: params.sender.id, + name: params.sender.name, + tag: params.sender.tag, + memberRoleIds: Object.freeze([...params.memberRoleIds]), + }); +} + +export function filterDiscordHistoryEntriesForContext(params: { + entries: readonly DiscordHistoryEntry[]; + mode: ContextVisibilityMode; + isSenderAllowed: (sender: DiscordHistorySenderProvenance) => boolean; +}): { entries: DiscordHistoryEntry[]; omitted: number } { + if (params.mode === "all") { + return { entries: [...params.entries], omitted: 0 }; + } + const filtered = filterSupplementalContextItems({ + items: params.entries, + mode: params.mode, + kind: "history", + isSenderAllowed: (entry) => + Boolean(entry.senderProvenance) && params.isSenderAllowed(entry.senderProvenance), + }); + return { entries: filtered.items, omitted: filtered.omitted }; +} diff --git a/extensions/discord/src/monitor/message-handler.preflight-history.ts b/extensions/discord/src/monitor/message-handler.preflight-history.ts index 03167aefb860..33c4fedc17e4 100644 --- a/extensions/discord/src/monitor/message-handler.preflight-history.ts +++ b/extensions/discord/src/monitor/message-handler.preflight-history.ts @@ -1,15 +1,21 @@ // Discord plugin module implements message handler.preflight history behavior. -import type { HistoryEntry } from "openclaw/plugin-sdk/reply-history"; import { resolveTimestampMs } from "./format.js"; +import { + createDiscordHistorySenderProvenance, + type DiscordHistoryEntry, +} from "./message-handler.history.js"; import type { DiscordMessagePreflightContext } from "./message-handler.preflight.types.js"; import { resolveDiscordMessageHistoryText } from "./message-utils.js"; +import type { DiscordSenderIdentity } from "./sender-identity.js"; export function buildDiscordPreflightHistoryEntry(params: { isGuildMessage: boolean; historyLimit: number; message: DiscordMessagePreflightContext["message"]; senderLabel: string; -}): HistoryEntry | undefined { + sender: Pick; + memberRoleIds: readonly string[]; +}): DiscordHistoryEntry | undefined { const textForHistory = resolveDiscordMessageHistoryText(params.message, { includeForwarded: true, }); @@ -19,6 +25,10 @@ export function buildDiscordPreflightHistoryEntry(params: { body: textForHistory, timestamp: resolveTimestampMs(params.message.timestamp), messageId: params.message.id, + senderProvenance: createDiscordHistorySenderProvenance({ + sender: params.sender, + memberRoleIds: params.memberRoleIds, + }), } : undefined; } diff --git a/extensions/discord/src/monitor/message-handler.preflight.test.ts b/extensions/discord/src/monitor/message-handler.preflight.test.ts index 9ffa1efb7446..33eac19db7fd 100644 --- a/extensions/discord/src/monitor/message-handler.preflight.test.ts +++ b/extensions/discord/src/monitor/message-handler.preflight.test.ts @@ -1890,6 +1890,10 @@ describe("preflightDiscordMessage", () => { sender: "Alice", body: "", messageId: "m-history-image", + senderProvenance: { + id: "user-1", + memberRoleIds: [], + }, media: [ { contentType: "image/png", diff --git a/extensions/discord/src/monitor/message-handler.preflight.ts b/extensions/discord/src/monitor/message-handler.preflight.ts index 21e23343c62c..5a0973e60fc5 100644 --- a/extensions/discord/src/monitor/message-handler.preflight.ts +++ b/extensions/discord/src/monitor/message-handler.preflight.ts @@ -8,7 +8,7 @@ import { recordChannelBotPairLoopAndCheckSuppression, resolveInboundMentionDecision, resolveUnmentionedGroupInboundPolicy, - recordDroppedChannelInboundHistory, + toHistoryMediaEntries, toInboundMediaFacts, } from "openclaw/plugin-sdk/channel-inbound"; import { isRecentOutboundMessageIdentity } from "openclaw/plugin-sdk/channel-outbound"; @@ -18,7 +18,7 @@ import { shouldHandleTextCommands } from "openclaw/plugin-sdk/command-surface"; import { isDangerousNameMatchingEnabled } from "openclaw/plugin-sdk/dangerous-name-runtime"; import { logDebug } from "openclaw/plugin-sdk/logging-core"; import { mimeTypeFromFilePath } from "openclaw/plugin-sdk/media-mime"; -import type { HistoryEntry } from "openclaw/plugin-sdk/reply-history"; +import { createChannelHistoryWindow } from "openclaw/plugin-sdk/reply-history"; import { getChildLogger, logVerbose } from "openclaw/plugin-sdk/runtime-env"; import { enqueueSystemEvent } from "openclaw/plugin-sdk/system-event-runtime"; import { resolveDefaultDiscordAccountId } from "../accounts.js"; @@ -33,6 +33,7 @@ import { resolveDiscordTextCommandAccess } from "./dm-command-auth.js"; import { resolveDiscordSystemLocation, resolveTimestampMs } from "./format.js"; import { resolveDiscordMessageStickers } from "./message-forwarded.js"; import { resolveDiscordDmPreflightAccess } from "./message-handler.dm-preflight.js"; +import type { DiscordHistoryEntry } from "./message-handler.history.js"; import { hydrateDiscordMessageIfNeeded } from "./message-handler.hydration.js"; import { resolveDiscordPreflightChannelAccess } from "./message-handler.preflight-channel-access.js"; import { resolveDiscordPreflightChannelContext } from "./message-handler.preflight-channel-context.js"; @@ -180,44 +181,28 @@ async function recordDiscordPendingHistoryEntry(params: { preflight: DiscordMessagePreflightParams; historyKey: string; message: DiscordMessagePreflightContext["message"]; - entry?: HistoryEntry; + entry?: DiscordHistoryEntry; }) { - if (params.preflight.historyLimit <= 0) { + if (!params.entry || params.preflight.historyLimit <= 0) { return; } - await recordDroppedChannelInboundHistory({ - input: { - id: params.message.id, - timestamp: params.entry?.timestamp, - rawText: params.entry?.body ?? "", - textForAgent: params.entry?.body, - raw: params.message, - }, - admission: { kind: "drop", reason: "discord-preflight", recordHistory: true }, - preflight: { - message: params.entry - ? { - rawBody: params.entry.body, - body: params.entry.body, - bodyForAgent: params.entry.body, - senderLabel: params.entry.sender, - envelopeFrom: params.entry.sender, - } - : undefined, - history: { - key: params.historyKey, - historyMap: params.preflight.guildHistories, - limit: params.preflight.historyLimit, - recordOnDrop: true, - mediaLimit: DISCORD_HISTORY_MEDIA_MAX_ATTACHMENTS, - shouldRecord: () => !isPreflightAborted(params.preflight.abortSignal), - }, - media: () => - resolveDiscordHistoryMediaForPendingRecord({ + await createChannelHistoryWindow({ + historyMap: params.preflight.guildHistories, + }).recordWithMedia({ + historyKey: params.historyKey, + entry: params.entry, + limit: params.preflight.historyLimit, + mediaLimit: DISCORD_HISTORY_MEDIA_MAX_ATTACHMENTS, + messageId: params.message.id, + shouldRecord: () => !isPreflightAborted(params.preflight.abortSignal), + media: async () => + toHistoryMediaEntries( + await resolveDiscordHistoryMediaForPendingRecord({ preflight: params.preflight, message: params.message, }), - }, + { messageId: params.message.id }, + ), }); } @@ -559,6 +544,8 @@ export async function preflightDiscordMessage( historyLimit: params.historyLimit, message, senderLabel: sender.label, + sender, + memberRoleIds, }); const threadOwnerId = threadChannel diff --git a/extensions/discord/src/monitor/message-handler.preflight.types.ts b/extensions/discord/src/monitor/message-handler.preflight.types.ts index fba0f8e76605..9e0903c4af98 100644 --- a/extensions/discord/src/monitor/message-handler.preflight.types.ts +++ b/extensions/discord/src/monitor/message-handler.preflight.types.ts @@ -2,11 +2,11 @@ import type { InboundEventKind } from "openclaw/plugin-sdk/channel-inbound"; import type { OpenClawConfig, ReplyToMode } from "openclaw/plugin-sdk/config-contracts"; import type { SessionBindingRecord } from "openclaw/plugin-sdk/conversation-runtime"; -import type { HistoryEntry } from "openclaw/plugin-sdk/reply-history"; import type { resolveAgentRoute } from "openclaw/plugin-sdk/routing"; import type { ChannelType, Client, User } from "../internal/discord.js"; import type { DiscordChannelConfigResolved, DiscordGuildEntryResolved } from "./allow-list.js"; import type { DiscordIngressLifecycle } from "./ingress.js"; +import type { DiscordHistoryEntry } from "./message-handler.history.js"; import type { DiscordChannelInfo, DiscordMediaInfo } from "./message-utils.js"; import type { DiscordThreadBindingLookup } from "./reply-delivery.js"; import type { DiscordSenderIdentity } from "./sender-identity.js"; @@ -29,7 +29,7 @@ type DiscordMessagePreflightSharedFields = { runtime: RuntimeEnv; botUserId?: string; abortSignal?: AbortSignal; - guildHistories: Map; + guildHistories: Map; historyLimit: number; mediaMaxBytes: number; textLimit: number; @@ -99,7 +99,7 @@ export type DiscordMessagePreflightContext = DiscordMessagePreflightSharedFields inboundEventKind: InboundEventKind; canDetectMention: boolean; - historyEntry?: HistoryEntry; + historyEntry?: DiscordHistoryEntry; threadBindings: DiscordThreadBindingLookup; discordRestFetch?: typeof fetch; }; diff --git a/extensions/discord/src/monitor/message-handler.process.room-events.test.ts b/extensions/discord/src/monitor/message-handler.process.room-events.test.ts index 5953da782d12..69187ef82b3d 100644 --- a/extensions/discord/src/monitor/message-handler.process.room-events.test.ts +++ b/extensions/discord/src/monitor/message-handler.process.room-events.test.ts @@ -112,6 +112,7 @@ describe("processDiscordMessage session routing and room events", () => { inboundEventKind: "room_event", baseSessionKey: BASE_CHANNEL_ROUTE.sessionKey, route: BASE_CHANNEL_ROUTE, + sender: { id: "U1", label: "user", name: "alice", isPluralKit: false }, }); await runProcessDiscordMessage(ctx); @@ -124,6 +125,10 @@ describe("processDiscordMessage session routing and room events", () => { body: "hi", messageId: "m1", sender: "Alice", + senderProvenance: { + id: "U1", + memberRoleIds: [], + }, }, ]); }); diff --git a/extensions/discord/src/monitor/message-handler.test-harness.ts b/extensions/discord/src/monitor/message-handler.test-harness.ts index 960589a97bfc..77c2f98000a4 100644 --- a/extensions/discord/src/monitor/message-handler.test-harness.ts +++ b/extensions/discord/src/monitor/message-handler.test-harness.ts @@ -20,7 +20,8 @@ export async function createBaseDiscordMessageContext( historyLimit: 0, mediaMaxBytes: 1024, textLimit: 4000, - sender: { label: "user" }, + sender: { id: "U1", label: "user", name: "alice", isPluralKit: false }, + memberRoleIds: [], replyToMode: "off", ackReactionScope: "group-mentions", groupPolicy: "open", diff --git a/extensions/discord/src/monitor/provider.ts b/extensions/discord/src/monitor/provider.ts index 891b0b4f7edd..7aea8c93c710 100644 --- a/extensions/discord/src/monitor/provider.ts +++ b/extensions/discord/src/monitor/provider.ts @@ -370,7 +370,7 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) { const logger = createSubsystemLogger("discord/monitor"); const guildHistories = new Map< string, - import("openclaw/plugin-sdk/reply-history").HistoryEntry[] + import("./message-handler.history.js").DiscordHistoryEntry[] >(); const { botUserId, botUserName } = await fetchDiscordBotIdentity({ client,