diff --git a/CHANGELOG.md b/CHANGELOG.md index eac5c3d2463..f7f8840f5f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ Docs: https://docs.openclaw.ai - Routing/session duplicate suppression synthesis: align shared session delivery-context inheritance, channel-paired route-field merges, and reply-surface target matching so dmScope=main turns avoid cross-surface duplicate replies while thread-aware forwarding keeps intended routing semantics. (from #33629, #26889, #17337, #33250) Thanks @Yuandiaodiaodiao, @kevinwildenradt, @Glucksberg, and @bmendonca3. - Routing/legacy session route inheritance: preserve external route metadata inheritance for legacy channel session keys (`agent:::` and `...:thread:`) so `chat.send` does not incorrectly fall back to webchat when valid delivery context exists. Follow-up to #33786. - Routing/legacy route guard tightening: require legacy session-key channel hints to match the saved delivery channel before inheriting external routing metadata, preventing custom namespaced keys like `agent::work:` from inheriting stale non-webchat routes. +- Gateway/internal client routing continuity: prevent webchat/TUI/UI turns from inheriting stale external reply routes by requiring explicit `deliver: true` for external delivery, keeping main-session external inheritance scoped to non-Webchat/UI clients, and honoring configured `session.mainKey` when identifying main-session continuity. (from #35321, #34635, #35356) Thanks @alexyyyander and @Octane0411. - Security/auth labels: remove token and API-key snippets from user-facing auth status labels so `/status` and `/models` do not expose credential fragments. (#33262) thanks @cu1ch3n. - Auth/credential semantics: align profile eligibility + probe diagnostics with SecretRef/expiry rules and harden browser download atomic writes. (#33733) thanks @joshavant. - Security/audit denyCommands guidance: suggest likely exact node command IDs for unknown `gateway.nodes.denyCommands` entries so ineffective denylist entries are easier to correct. (#29713) thanks @liquidhorizon88-bot. diff --git a/src/gateway/server-methods/chat.directive-tags.test.ts b/src/gateway/server-methods/chat.directive-tags.test.ts index 0ea0e0181c2..f9acd15805e 100644 --- a/src/gateway/server-methods/chat.directive-tags.test.ts +++ b/src/gateway/server-methods/chat.directive-tags.test.ts @@ -4,12 +4,13 @@ import path from "node:path"; 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 } from "../protocol/client-info.js"; +import { GATEWAY_CLIENT_CAPS, GATEWAY_CLIENT_MODES } from "../protocol/client-info.js"; import type { GatewayRequestContext } from "./types.js"; const mockState = vi.hoisted(() => ({ transcriptPath: "", sessionId: "sess-1", + mainSessionKey: "main", finalText: "[[reply_to_current]]", triggerAgentRunStart: false, agentRunId: "run-agent-1", @@ -31,7 +32,11 @@ vi.mock("../session-utils.js", async (importOriginal) => { return { ...original, loadSessionEntry: (rawKey: string) => ({ - cfg: {}, + cfg: { + session: { + mainKey: mockState.mainSessionKey, + }, + }, storePath: path.join(path.dirname(mockState.transcriptPath), "sessions.json"), entry: { sessionId: mockState.sessionId, @@ -148,15 +153,25 @@ async function runNonStreamingChatSend(params: { idempotencyKey: string; message?: string; sessionKey?: string; + deliver?: boolean; client?: unknown; expectBroadcast?: boolean; }) { + const sendParams: { + sessionKey: string; + message: string; + idempotencyKey: string; + deliver?: boolean; + } = { + sessionKey: params.sessionKey ?? "main", + message: params.message ?? "hello", + idempotencyKey: params.idempotencyKey, + }; + if (typeof params.deliver === "boolean") { + sendParams.deliver = params.deliver; + } await chatHandlers["chat.send"]({ - params: { - sessionKey: params.sessionKey ?? "main", - message: params.message ?? "hello", - idempotencyKey: params.idempotencyKey, - }, + params: sendParams, respond: params.respond as unknown as Parameters< (typeof chatHandlers)["chat.send"] >[0]["respond"], @@ -190,6 +205,7 @@ async function runNonStreamingChatSend(params: { describe("chat directive tag stripping for non-streaming final payloads", () => { afterEach(() => { mockState.finalText = "[[reply_to_current]]"; + mockState.mainSessionKey = "main"; mockState.triggerAgentRunStart = false; mockState.agentRunId = "run-agent-1"; mockState.sessionEntry = {}; @@ -369,6 +385,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => respond, idempotencyKey: "idem-origin-routing", sessionKey: "agent:main:telegram:direct:6812765697", + deliver: true, expectBroadcast: false, }); @@ -403,6 +420,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => respond, idempotencyKey: "idem-feishu-origin-routing", sessionKey: "agent:main:feishu:direct:ou_feishu_direct_123", + deliver: true, expectBroadcast: false, }); @@ -436,6 +454,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => respond, idempotencyKey: "idem-per-account-channel-peer-routing", sessionKey: "agent:main:telegram:account-a:direct:6812765697", + deliver: true, expectBroadcast: false, }); @@ -469,6 +488,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => respond, idempotencyKey: "idem-legacy-channel-peer-routing", sessionKey: "agent:main:telegram:6812765697", + deliver: true, expectBroadcast: false, }); @@ -504,6 +524,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () => respond, idempotencyKey: "idem-legacy-thread-channel-peer-routing", sessionKey: "agent:main:telegram:6812765697:thread:42", + deliver: true, expectBroadcast: false, }); @@ -550,6 +571,90 @@ describe("chat directive tag stripping for non-streaming final payloads", () => ); }); + it("chat.send does not inherit external delivery context for UI clients on main sessions", async () => { + createTranscriptFixture("openclaw-chat-send-main-ui-routes-"); + 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-main-ui-routes", + client: { + connect: { + client: { + mode: GATEWAY_CLIENT_MODES.UI, + id: "openclaw-tui", + }, + }, + } as unknown, + sessionKey: "agent:main:main", + expectBroadcast: false, + }); + + expect(mockState.lastDispatchCtx).toEqual( + expect.objectContaining({ + OriginatingChannel: "webchat", + OriginatingTo: undefined, + AccountId: undefined, + }), + ); + }); + + it("chat.send inherits external delivery context for CLI clients on configured main sessions", async () => { + createTranscriptFixture("openclaw-chat-send-config-main-cli-routes-"); + 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-cli-routes", + client: { + connect: { + client: { + mode: GATEWAY_CLIENT_MODES.CLI, + id: "cli", + }, + }, + } 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"; @@ -584,4 +689,38 @@ describe("chat directive tag stripping for non-streaming final payloads", () => }), ); }); + + it("chat.send keeps replies on the internal surface when deliver is not enabled", async () => { + createTranscriptFixture("openclaw-chat-send-no-deliver-internal-surface-"); + mockState.finalText = "ok"; + mockState.sessionEntry = { + deliveryContext: { + channel: "discord", + to: "user:1234567890", + accountId: "default", + }, + lastChannel: "discord", + lastTo: "user:1234567890", + lastAccountId: "default", + }; + const respond = vi.fn(); + const context = createChatContext(); + + await runNonStreamingChatSend({ + context, + respond, + idempotencyKey: "idem-no-deliver-internal-surface", + sessionKey: "agent:main:discord:direct:1234567890", + deliver: false, + expectBroadcast: false, + }); + + expect(mockState.lastDispatchCtx).toEqual( + expect.objectContaining({ + OriginatingChannel: "webchat", + OriginatingTo: undefined, + AccountId: undefined, + }), + ); + }); }); diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts index 13feee2d131..1c750ec0db6 100644 --- a/src/gateway/server-methods/chat.ts +++ b/src/gateway/server-methods/chat.ts @@ -17,7 +17,11 @@ import { stripInlineDirectiveTagsForDisplay, stripInlineDirectiveTagsFromMessageForDisplay, } from "../../utils/directive-tags.js"; -import { INTERNAL_MESSAGE_CHANNEL, normalizeMessageChannel } from "../../utils/message-channel.js"; +import { + INTERNAL_MESSAGE_CHANNEL, + isWebchatClient, + normalizeMessageChannel, +} from "../../utils/message-channel.js"; import { abortChatRunById, abortChatRunsForSessionKey, @@ -28,7 +32,11 @@ import { } from "../chat-abort.js"; import { type ChatImageContent, parseMessageWithAttachments } from "../chat-attachments.js"; import { stripEnvelopeFromMessage, stripEnvelopeFromMessages } from "../chat-sanitize.js"; -import { GATEWAY_CLIENT_CAPS, hasGatewayClientCap } from "../protocol/client-info.js"; +import { + GATEWAY_CLIENT_CAPS, + GATEWAY_CLIENT_MODES, + hasGatewayClientCap, +} from "../protocol/client-info.js"; import { ErrorCodes, errorShape, @@ -856,6 +864,7 @@ 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, ); @@ -867,11 +876,12 @@ export const chatHandlers: GatewayRequestHandlers = { 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( - (sessionScopeHead ?? "").trim().toLowerCase(), + normalizedSessionScopeHead, ); const isChannelScopedSession = sessionPeerShapeCandidates.some((part) => CHANNEL_SCOPED_SESSION_SHAPES.has(part), @@ -880,16 +890,24 @@ export const chatHandlers: GatewayRequestHandlers = { !isChannelScopedSession && typeof sessionScopeParts[1] === "string" && sessionChannelHint === routeChannelCandidate; - // Only inherit prior external route metadata for channel-scoped sessions. - // Channel-agnostic sessions (main, direct:, etc.) can otherwise - // leak stale routes across surfaces. + 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), + ((!isChannelAgnosticSessionScope && + (isChannelScopedSession || hasLegacyChannelPeerShape)) || + (isConfiguredMainSessionScope && client?.connect !== undefined && !isFromWebchatClient)), ); const hasDeliverableRoute = + shouldDeliverExternally && canInheritDeliverableRoute && routeChannelCandidate && routeChannelCandidate !== INTERNAL_MESSAGE_CHANNEL &&