diff --git a/CHANGELOG.md b/CHANGELOG.md index 42f187ad8..9f5c74cc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -204,6 +204,7 @@ Docs: https://docs.openclaw.ai - Agents/gateway config guidance: stop exposing `config.schema` through the agent `gateway` tool, remove prompt/docs guidance that told agents to call it, and keep agents on `config.get` plus `config.patch`/`config.apply` for config changes. (#7382) thanks @kakuteki. - Agents/failover: classify periodic provider limit exhaustion text (for example `Weekly/Monthly Limit Exhausted`) as `rate_limit` while keeping explicit `402 Payment Required` variants in billing, so failover continues without misclassifying billing-wrapped quota errors. (#33813) thanks @zhouhe-xydt. - Mattermost/interactive button callbacks: allow external callback base URLs and stop requiring loopback-origin requests so button clicks work when Mattermost reaches the gateway over Tailscale, LAN, or a reverse proxy. (#37543) thanks @mukhtharcm. +- Gateway/chat.send route inheritance: keep explicit external delivery for channel-scoped sessions while preventing shared-main and other channel-agnostic webchat sessions from inheriting stale external routes, so Control UI replies stay on webchat without breaking selected channel-target sessions. (#34669) Thanks @vincentkoc. - Telegram/Discord media upload caps: make outbound uploads honor channel `mediaMaxMb` config, raise Telegram's default media cap to 100MB, and remove MIME fallback limits that kept some Telegram uploads at 16MB. Thanks @vincentkoc. - Skills/nano-banana-pro resolution override: respect explicit `--resolution` values during image editing and only auto-detect output size from input images when the flag is omitted. (#36880) Thanks @shuofengzhang and @vincentkoc. - Skills/openai-image-gen CLI validation: validate `--background` and `--style` inputs early, normalize supported values, and warn when those flags are ignored for incompatible models. (#36762) Thanks @shuofengzhang and @vincentkoc. diff --git a/src/gateway/protocol/schema/logs-chat.ts b/src/gateway/protocol/schema/logs-chat.ts index b8d0fe1ba..ffa01945c 100644 --- a/src/gateway/protocol/schema/logs-chat.ts +++ b/src/gateway/protocol/schema/logs-chat.ts @@ -1,5 +1,5 @@ import { Type } from "@sinclair/typebox"; -import { NonEmptyString } from "./primitives.js"; +import { ChatSendSessionKeyString, NonEmptyString } from "./primitives.js"; export const LogsTailParamsSchema = Type.Object( { @@ -33,7 +33,7 @@ export const ChatHistoryParamsSchema = Type.Object( export const ChatSendParamsSchema = Type.Object( { - sessionKey: NonEmptyString, + sessionKey: ChatSendSessionKeyString, message: Type.String(), thinking: Type.Optional(Type.String()), deliver: Type.Optional(Type.Boolean()), diff --git a/src/gateway/protocol/schema/primitives.ts b/src/gateway/protocol/schema/primitives.ts index d43a16a1e..849778149 100644 --- a/src/gateway/protocol/schema/primitives.ts +++ b/src/gateway/protocol/schema/primitives.ts @@ -3,6 +3,11 @@ import { SESSION_LABEL_MAX_LENGTH } from "../../../sessions/session-label.js"; import { GATEWAY_CLIENT_IDS, GATEWAY_CLIENT_MODES } from "../client-info.js"; export const NonEmptyString = Type.String({ minLength: 1 }); +export const CHAT_SEND_SESSION_KEY_MAX_LENGTH = 512; +export const ChatSendSessionKeyString = Type.String({ + minLength: 1, + maxLength: CHAT_SEND_SESSION_KEY_MAX_LENGTH, +}); export const SessionLabelString = Type.String({ minLength: 1, maxLength: SESSION_LABEL_MAX_LENGTH, diff --git a/src/gateway/server-methods/chat.directive-tags.test.ts b/src/gateway/server-methods/chat.directive-tags.test.ts index d4f631a21..717c81337 100644 --- a/src/gateway/server-methods/chat.directive-tags.test.ts +++ b/src/gateway/server-methods/chat.directive-tags.test.ts @@ -5,6 +5,8 @@ import { CURRENT_SESSION_VERSION } from "@mariozechner/pi-coding-agent"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { MsgContext } from "../../auto-reply/templating.js"; import { GATEWAY_CLIENT_CAPS, GATEWAY_CLIENT_MODES } from "../protocol/client-info.js"; +import { ErrorCodes } from "../protocol/index.js"; +import { CHAT_SEND_SESSION_KEY_MAX_LENGTH } from "../protocol/schema/primitives.js"; import type { GatewayRequestContext } from "./types.js"; const mockState = vi.hoisted(() => ({ @@ -325,6 +327,34 @@ describe("chat directive tag stripping for non-streaming final payloads", () => expect(extractFirstTextBlock(payload)).toBe(""); }); + it("rejects oversized chat.send session keys before dispatch", async () => { + createTranscriptFixture("openclaw-chat-send-session-key-too-long-"); + const respond = vi.fn(); + const context = createChatContext(); + + await chatHandlers["chat.send"]({ + params: { + sessionKey: `agent:main:${"x".repeat(CHAT_SEND_SESSION_KEY_MAX_LENGTH)}`, + message: "hello", + idempotencyKey: "idem-session-key-too-long", + }, + respond, + req: {} as never, + client: null as never, + isWebchatConnect: () => false, + context: context as GatewayRequestContext, + }); + + expect(respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ + code: ErrorCodes.INVALID_REQUEST, + }), + ); + expect(context.broadcast).not.toHaveBeenCalled(); + }); + it("chat.inject strips external untrusted wrapper metadata from final payload text", async () => { createTranscriptFixture("openclaw-chat-inject-untrusted-meta-"); const respond = vi.fn(); @@ -362,7 +392,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => expect(extractFirstTextBlock(payload)).toBe("hello"); }); - it("chat.send inherits originating routing metadata from session delivery context", async () => { + it("chat.send keeps explicit delivery routes for channel-scoped sessions", async () => { createTranscriptFixture("openclaw-chat-send-origin-routing-"); mockState.finalText = "ok"; mockState.sessionEntry = { @@ -400,7 +430,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => ); }); - it("chat.send inherits Feishu routing metadata from session delivery context", async () => { + it("chat.send keeps explicit delivery routes for Feishu channel-scoped sessions", async () => { createTranscriptFixture("openclaw-chat-send-feishu-origin-routing-"); mockState.finalText = "ok"; mockState.sessionEntry = { @@ -429,12 +459,13 @@ describe("chat directive tag stripping for non-streaming final payloads", () => expect.objectContaining({ OriginatingChannel: "feishu", OriginatingTo: "ou_feishu_direct_123", + ExplicitDeliverRoute: true, AccountId: "default", }), ); }); - it("chat.send inherits routing metadata for per-account channel-peer session keys", async () => { + it("chat.send keeps explicit delivery routes for per-account channel-peer sessions", async () => { createTranscriptFixture("openclaw-chat-send-per-account-channel-peer-routing-"); mockState.finalText = "ok"; mockState.sessionEntry = { @@ -463,12 +494,13 @@ describe("chat directive tag stripping for non-streaming final payloads", () => expect.objectContaining({ OriginatingChannel: "telegram", OriginatingTo: "telegram:6812765697", + ExplicitDeliverRoute: true, AccountId: "account-a", }), ); }); - it("chat.send inherits routing metadata for legacy channel-peer session keys", async () => { + it("chat.send keeps explicit delivery routes for legacy channel-peer sessions", async () => { createTranscriptFixture("openclaw-chat-send-legacy-channel-peer-routing-"); mockState.finalText = "ok"; mockState.sessionEntry = { @@ -497,12 +529,13 @@ describe("chat directive tag stripping for non-streaming final payloads", () => expect.objectContaining({ OriginatingChannel: "telegram", OriginatingTo: "telegram:6812765697", + ExplicitDeliverRoute: true, AccountId: "default", }), ); }); - it("chat.send inherits routing metadata for legacy channel-peer thread session keys", async () => { + it("chat.send keeps explicit delivery routes for legacy thread sessions", async () => { createTranscriptFixture("openclaw-chat-send-legacy-thread-channel-peer-routing-"); mockState.finalText = "ok"; mockState.sessionEntry = { @@ -533,6 +566,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => expect.objectContaining({ OriginatingChannel: "telegram", OriginatingTo: "telegram:6812765697", + ExplicitDeliverRoute: true, AccountId: "default", MessageThreadId: "42", }), @@ -657,6 +691,44 @@ describe("chat directive tag stripping for non-streaming final payloads", () => ); }); + it("chat.send keeps configured main delivery inheritance when connect metadata omits client details", async () => { + createTranscriptFixture("openclaw-chat-send-config-main-connect-no-client-"); + mockState.mainSessionKey = "work"; + mockState.finalText = "ok"; + mockState.sessionEntry = { + deliveryContext: { + channel: "whatsapp", + to: "whatsapp:+8613800138000", + accountId: "default", + }, + lastChannel: "whatsapp", + lastTo: "whatsapp:+8613800138000", + lastAccountId: "default", + }; + const respond = vi.fn(); + const context = createChatContext(); + + await runNonStreamingChatSend({ + context, + respond, + idempotencyKey: "idem-config-main-connect-no-client", + client: { + connect: {}, + } as unknown, + sessionKey: "agent:main:work", + deliver: true, + expectBroadcast: false, + }); + + expect(mockState.lastDispatchCtx).toEqual( + expect.objectContaining({ + OriginatingChannel: "whatsapp", + OriginatingTo: "whatsapp:+8613800138000", + AccountId: "default", + }), + ); + }); + it("chat.send does not inherit external delivery context for non-channel custom sessions", async () => { createTranscriptFixture("openclaw-chat-send-custom-no-cross-route-"); mockState.finalText = "ok"; diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts index e384006ae..497902b63 100644 --- a/src/gateway/server-methods/chat.ts +++ b/src/gateway/server-methods/chat.ts @@ -46,6 +46,7 @@ import { validateChatInjectParams, validateChatSendParams, } from "../protocol/index.js"; +import { CHAT_SEND_SESSION_KEY_MAX_LENGTH } from "../protocol/schema/primitives.js"; import { getMaxChatHistoryMessagesBytes } from "../server-constants.js"; import { capArrayByJsonBytes, @@ -95,6 +96,118 @@ const CHANNEL_AGNOSTIC_SESSION_SCOPES = new Set([ ]); const CHANNEL_SCOPED_SESSION_SHAPES = new Set(["direct", "dm", "group", "channel"]); +type ChatSendDeliveryEntry = { + deliveryContext?: { + channel?: string; + to?: string; + accountId?: string; + threadId?: string | number; + }; + lastChannel?: string; + lastTo?: string; + lastAccountId?: string; + lastThreadId?: string | number; +}; + +type ChatSendOriginatingRoute = { + originatingChannel: string; + originatingTo?: string; + accountId?: string; + messageThreadId?: string | number; + explicitDeliverRoute: boolean; +}; + +function resolveChatSendOriginatingRoute(params: { + client?: { mode?: string | null; id?: string | null } | null; + deliver?: boolean; + entry?: ChatSendDeliveryEntry; + hasConnectedClient?: boolean; + mainKey?: string; + sessionKey: string; +}): ChatSendOriginatingRoute { + const shouldDeliverExternally = params.deliver === true; + if (!shouldDeliverExternally) { + return { + originatingChannel: INTERNAL_MESSAGE_CHANNEL, + explicitDeliverRoute: false, + }; + } + + const routeChannelCandidate = normalizeMessageChannel( + params.entry?.deliveryContext?.channel ?? params.entry?.lastChannel, + ); + const routeToCandidate = params.entry?.deliveryContext?.to ?? params.entry?.lastTo; + const routeAccountIdCandidate = + params.entry?.deliveryContext?.accountId ?? params.entry?.lastAccountId ?? undefined; + const routeThreadIdCandidate = + params.entry?.deliveryContext?.threadId ?? params.entry?.lastThreadId; + if (params.sessionKey.length > CHAT_SEND_SESSION_KEY_MAX_LENGTH) { + return { + originatingChannel: INTERNAL_MESSAGE_CHANNEL, + explicitDeliverRoute: false, + }; + } + + const parsedSessionKey = parseAgentSessionKey(params.sessionKey); + const sessionScopeParts = (parsedSessionKey?.rest ?? params.sessionKey) + .split(":", 3) + .filter(Boolean); + const sessionScopeHead = sessionScopeParts[0]; + const sessionChannelHint = normalizeMessageChannel(sessionScopeHead); + const normalizedSessionScopeHead = (sessionScopeHead ?? "").trim().toLowerCase(); + const sessionPeerShapeCandidates = [sessionScopeParts[1], sessionScopeParts[2]] + .map((part) => (part ?? "").trim().toLowerCase()) + .filter(Boolean); + const isChannelAgnosticSessionScope = CHANNEL_AGNOSTIC_SESSION_SCOPES.has( + normalizedSessionScopeHead, + ); + const isChannelScopedSession = sessionPeerShapeCandidates.some((part) => + CHANNEL_SCOPED_SESSION_SHAPES.has(part), + ); + const hasLegacyChannelPeerShape = + !isChannelScopedSession && + typeof sessionScopeParts[1] === "string" && + sessionChannelHint === routeChannelCandidate; + const isFromWebchatClient = + isWebchatClient(params.client) || params.client?.mode === GATEWAY_CLIENT_MODES.UI; + const configuredMainKey = (params.mainKey ?? "main").trim().toLowerCase(); + const isConfiguredMainSessionScope = + normalizedSessionScopeHead.length > 0 && normalizedSessionScopeHead === configuredMainKey; + + // Keep explicit delivery for channel-scoped sessions, but refuse to inherit + // stale external routes for shared-main and other channel-agnostic webchat/UI + // turns where the session key does not encode the user's current target. + // Preserve the old configured-main contract: any connected non-webchat client + // may inherit the last external route even when client metadata is absent. + const canInheritDeliverableRoute = Boolean( + sessionChannelHint && + sessionChannelHint !== INTERNAL_MESSAGE_CHANNEL && + ((!isChannelAgnosticSessionScope && (isChannelScopedSession || hasLegacyChannelPeerShape)) || + (isConfiguredMainSessionScope && params.hasConnectedClient && !isFromWebchatClient)), + ); + const hasDeliverableRoute = + canInheritDeliverableRoute && + routeChannelCandidate && + routeChannelCandidate !== INTERNAL_MESSAGE_CHANNEL && + typeof routeToCandidate === "string" && + routeToCandidate.trim().length > 0; + + if (!hasDeliverableRoute) { + return { + originatingChannel: INTERNAL_MESSAGE_CHANNEL, + explicitDeliverRoute: false, + }; + } + + return { + originatingChannel: routeChannelCandidate, + originatingTo: routeToCandidate, + accountId: routeAccountIdCandidate, + messageThreadId: routeThreadIdCandidate, + explicitDeliverRoute: true, + }; +} + function stripDisallowedChatControlChars(message: string): string { let output = ""; for (const char of message) { @@ -864,62 +977,20 @@ export const chatHandlers: GatewayRequestHandlers = { ); const commandBody = injectThinking ? `/think ${p.thinking} ${parsedMessage}` : parsedMessage; const clientInfo = client?.connect?.client; - const shouldDeliverExternally = p.deliver === true; - const routeChannelCandidate = normalizeMessageChannel( - entry?.deliveryContext?.channel ?? entry?.lastChannel, - ); - const routeToCandidate = entry?.deliveryContext?.to ?? entry?.lastTo; - const routeAccountIdCandidate = - entry?.deliveryContext?.accountId ?? entry?.lastAccountId ?? undefined; - const routeThreadIdCandidate = entry?.deliveryContext?.threadId ?? entry?.lastThreadId; - const parsedSessionKey = parseAgentSessionKey(sessionKey); - const sessionScopeParts = (parsedSessionKey?.rest ?? sessionKey).split(":").filter(Boolean); - const sessionScopeHead = sessionScopeParts[0]; - const sessionChannelHint = normalizeMessageChannel(sessionScopeHead); - const normalizedSessionScopeHead = (sessionScopeHead ?? "").trim().toLowerCase(); - const sessionPeerShapeCandidates = [sessionScopeParts[1], sessionScopeParts[2]] - .map((part) => (part ?? "").trim().toLowerCase()) - .filter(Boolean); - const isChannelAgnosticSessionScope = CHANNEL_AGNOSTIC_SESSION_SCOPES.has( - normalizedSessionScopeHead, - ); - const isChannelScopedSession = sessionPeerShapeCandidates.some((part) => - CHANNEL_SCOPED_SESSION_SHAPES.has(part), - ); - const hasLegacyChannelPeerShape = - !isChannelScopedSession && - typeof sessionScopeParts[1] === "string" && - sessionChannelHint === routeChannelCandidate; - const clientMode = client?.connect?.client?.mode; - const isFromWebchatClient = - isWebchatClient(client?.connect?.client) || clientMode === GATEWAY_CLIENT_MODES.UI; - const configuredMainKey = (cfg.session?.mainKey ?? "main").trim().toLowerCase(); - const isConfiguredMainSessionScope = - normalizedSessionScopeHead.length > 0 && normalizedSessionScopeHead === configuredMainKey; - // Channel-agnostic session scopes (main, direct:, etc.) can leak - // stale routes across surfaces. Allow configured main sessions from - // non-Webchat/UI clients (e.g., CLI, backend) to keep the last external route. - const canInheritDeliverableRoute = Boolean( - sessionChannelHint && - sessionChannelHint !== INTERNAL_MESSAGE_CHANNEL && - ((!isChannelAgnosticSessionScope && - (isChannelScopedSession || hasLegacyChannelPeerShape)) || - (isConfiguredMainSessionScope && client?.connect !== undefined && !isFromWebchatClient)), - ); - const hasDeliverableRoute = Boolean( - shouldDeliverExternally && - canInheritDeliverableRoute && - routeChannelCandidate && - routeChannelCandidate !== INTERNAL_MESSAGE_CHANNEL && - typeof routeToCandidate === "string" && - routeToCandidate.trim().length > 0, - ); - const originatingChannel = hasDeliverableRoute - ? routeChannelCandidate - : INTERNAL_MESSAGE_CHANNEL; - const originatingTo = hasDeliverableRoute ? routeToCandidate : undefined; - const accountId = hasDeliverableRoute ? routeAccountIdCandidate : undefined; - const messageThreadId = hasDeliverableRoute ? routeThreadIdCandidate : undefined; + const { + originatingChannel, + originatingTo, + accountId, + messageThreadId, + explicitDeliverRoute, + } = resolveChatSendOriginatingRoute({ + client: clientInfo, + deliver: p.deliver, + entry, + hasConnectedClient: client?.connect !== undefined, + mainKey: cfg.session?.mainKey, + sessionKey, + }); // Inject timestamp so agents know the current date/time. // Only BodyForAgent gets the timestamp — Body stays raw for UI display. // See: https://github.com/moltbot/moltbot/issues/3658 @@ -936,7 +1007,7 @@ export const chatHandlers: GatewayRequestHandlers = { Surface: INTERNAL_MESSAGE_CHANNEL, OriginatingChannel: originatingChannel, OriginatingTo: originatingTo, - ExplicitDeliverRoute: hasDeliverableRoute, + ExplicitDeliverRoute: explicitDeliverRoute, AccountId: accountId, MessageThreadId: messageThreadId, ChatType: "direct",