From 59f566547bf1993994792dc01fc2bc1c14413ff5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 25 Aug 2026 01:41:59 -0700 Subject: [PATCH] refactor(auto-reply): unify session delivery route resolution (#129124) --- src/auto-reply/reply/session-delivery.test.ts | 48 +++------ src/auto-reply/reply/session-delivery.ts | 102 ++++++------------ src/auto-reply/reply/session.ts | 18 +--- 3 files changed, 55 insertions(+), 113 deletions(-) diff --git a/src/auto-reply/reply/session-delivery.test.ts b/src/auto-reply/reply/session-delivery.test.ts index ee57835692cf..b2b0188ce8b0 100644 --- a/src/auto-reply/reply/session-delivery.test.ts +++ b/src/auto-reply/reply/session-delivery.test.ts @@ -1,33 +1,33 @@ // Tests how session delivery preserves previous channel and target routing state. import { describe, expect, it } from "vitest"; -import { resolveLastChannelRaw, resolveLastToRaw } from "./session-delivery.js"; +import { resolveSessionDeliveryRoute } from "./session-delivery.js"; describe("inter-session lastRoute preservation (fixes #54441)", () => { it("inter-session message does NOT overwrite established Discord lastChannel", () => { expect( - resolveLastChannelRaw({ + resolveSessionDeliveryRoute({ originatingChannelRaw: "webchat", persistedLastChannel: "discord", sessionKey: "agent:samantha:main", isInterSession: true, - }), + }).channel, ).toBe("discord"); }); it("inter-session message does NOT overwrite established Telegram lastChannel", () => { expect( - resolveLastChannelRaw({ + resolveSessionDeliveryRoute({ originatingChannelRaw: "webchat", persistedLastChannel: "telegram", sessionKey: "agent:main:telegram:direct:123456", isInterSession: true, - }), + }).channel, ).toBe("telegram"); }); it("inter-session message does NOT overwrite established external lastTo", () => { expect( - resolveLastToRaw({ + resolveSessionDeliveryRoute({ originatingChannelRaw: "webchat", originatingToRaw: "session:somekey", toRaw: "session:somekey", @@ -35,36 +35,36 @@ describe("inter-session lastRoute preservation (fixes #54441)", () => { persistedLastChannel: "discord", sessionKey: "agent:samantha:main", isInterSession: true, - }), + }).to, ).toBe("channel:1234567890"); }); it("regular Discord user message DOES update lastChannel normally", () => { expect( - resolveLastChannelRaw({ + resolveSessionDeliveryRoute({ originatingChannelRaw: "discord", persistedLastChannel: "discord", sessionKey: "agent:main:discord:channel:123", isInterSession: false, - }), + }).channel, ).toBe("discord"); }); it("inter-session on a NEW session (no persisted external route) may set webchat", () => { // When there is no established external route, inter-session should not // forcefully block the update — the session has no external route to protect. - const result = resolveLastChannelRaw({ + const result = resolveSessionDeliveryRoute({ originatingChannelRaw: "webchat", persistedLastChannel: undefined, sessionKey: "agent:samantha:main", isInterSession: true, - }); + }).channel; // No external route existed — falls through to normal resolution (webchat or undefined). expect(["webchat", undefined]).toContain(result); }); it("inter-session on session with no persisted lastTo preserves session route", () => { - const result = resolveLastToRaw({ + const result = resolveSessionDeliveryRoute({ originatingChannelRaw: "webchat", originatingToRaw: "session:somekey", toRaw: "session:somekey", @@ -72,7 +72,7 @@ describe("inter-session lastRoute preservation (fixes #54441)", () => { persistedLastChannel: undefined, sessionKey: "agent:samantha:main", isInterSession: true, - }); + }).to; // No external route — falls through to normal resolution expect(["session:somekey", undefined]).toContain(result); }); @@ -92,21 +92,14 @@ describe("session delivery direct-session routing overrides", () => { // Webchat/dashboard viewing an external-channel session must not overwrite // the delivery route — subagents must still deliver to the original channel. expect( - resolveLastChannelRaw({ - originatingChannelRaw: "webchat", - persistedLastChannel: "telegram", - sessionKey, - }), - ).toBe("telegram"); - expect( - resolveLastToRaw({ + resolveSessionDeliveryRoute({ originatingChannelRaw: "webchat", originatingToRaw: "session:dashboard", persistedLastChannel: "telegram", persistedLastTo: "123456", sessionKey, }), - ).toBe("123456"); + ).toEqual({ channel: "telegram", to: "123456" }); }, ); @@ -119,20 +112,13 @@ describe("session delivery direct-session routing overrides", () => { "agent:main:telegram:direct:123456:cron:job-1", ])("keeps persisted external routes for malformed direct-like key %s", (sessionKey) => { expect( - resolveLastChannelRaw({ - originatingChannelRaw: "webchat", - persistedLastChannel: "telegram", - sessionKey, - }), - ).toBe("telegram"); - expect( - resolveLastToRaw({ + resolveSessionDeliveryRoute({ originatingChannelRaw: "webchat", originatingToRaw: "session:dashboard", persistedLastChannel: "telegram", persistedLastTo: "group:12345", sessionKey, }), - ).toBe("group:12345"); + ).toEqual({ channel: "telegram", to: "group:12345" }); }); }); diff --git a/src/auto-reply/reply/session-delivery.ts b/src/auto-reply/reply/session-delivery.ts index 3c056a1e20d2..2d7488950251 100644 --- a/src/auto-reply/reply/session-delivery.ts +++ b/src/auto-reply/reply/session-delivery.ts @@ -94,52 +94,7 @@ function isExternalRoutingChannel(channel?: string): channel is string { ); } -export function resolveLastChannelRaw(params: { - originatingChannelRaw?: string; - persistedLastChannel?: string; - sessionKey?: string; - isInterSession?: boolean; -}): string | undefined { - const originatingChannel = normalizeMessageChannel(params.originatingChannelRaw); - // WebChat should own reply routing for direct-session UI turns, but only when - // the session has no established external delivery route. If the session was - // created via an external channel (e.g. Telegram, iMessage), webchat/dashboard - // access must not overwrite the persisted route — doing so causes subagent - // completion events to be delivered to the dashboard instead of the original - // channel. See: https://github.com/openclaw/openclaw/issues/47745 - const persistedChannel = normalizeMessageChannel(params.persistedLastChannel); - const sessionKeyChannelHint = resolveSessionKeyChannelHint(params.sessionKey); - const hasEstablishedExternalRoute = - isExternalRoutingChannel(persistedChannel) || isExternalRoutingChannel(sessionKeyChannelHint); - // Inter-session messages (sessions_send) always arrive with channel=webchat, - // but must never overwrite an already-established external delivery route. - // Without this guard, a sessions_send call resets lastChannel to webchat, - // causing subsequent Discord (or other external) deliveries to be lost. - // See: https://github.com/openclaw/openclaw/issues/54441 - if (params.isInterSession && hasEstablishedExternalRoute) { - return persistedChannel || sessionKeyChannelHint; - } - if ( - originatingChannel === INTERNAL_MESSAGE_CHANNEL && - !hasEstablishedExternalRoute && - (isMainSessionKey(params.sessionKey) || isDirectSessionKey(params.sessionKey)) - ) { - return params.originatingChannelRaw; - } - let resolved = params.originatingChannelRaw || params.persistedLastChannel; - // Internal/non-deliverable sources should not overwrite previously known - // external delivery routes (or explicit channel hints from the session key). - if (!isExternalRoutingChannel(originatingChannel)) { - if (isExternalRoutingChannel(persistedChannel)) { - resolved = persistedChannel; - } else if (isExternalRoutingChannel(sessionKeyChannelHint)) { - resolved = sessionKeyChannelHint; - } - } - return resolved; -} - -export function resolveLastToRaw(params: { +export function resolveSessionDeliveryRoute(params: { originatingChannelRaw?: string; originatingToRaw?: string; toRaw?: string; @@ -147,38 +102,47 @@ export function resolveLastToRaw(params: { persistedLastChannel?: string; sessionKey?: string; isInterSession?: boolean; -}): string | undefined { +}): { channel: string | undefined; to: string | undefined } { const originatingChannel = normalizeMessageChannel(params.originatingChannelRaw); const persistedChannel = normalizeMessageChannel(params.persistedLastChannel); const sessionKeyChannelHint = resolveSessionKeyChannelHint(params.sessionKey); - const hasEstablishedExternalRouteForTo = - isExternalRoutingChannel(persistedChannel) || isExternalRoutingChannel(sessionKeyChannelHint); - // Inter-session messages must not replace a persisted external `to` with - // webchat-scoped identifiers (e.g. session keys). Preserve the established - // external destination so deliveries continue routing to the correct channel. - // See: https://github.com/openclaw/openclaw/issues/54441 - if (params.isInterSession && hasEstablishedExternalRouteForTo && params.persistedLastTo) { - return params.persistedLastTo; - } + const establishedExternalChannel = isExternalRoutingChannel(persistedChannel) + ? persistedChannel + : isExternalRoutingChannel(sessionKeyChannelHint) + ? sessionKeyChannelHint + : undefined; + + // Webchat can own a direct/main session only before an external route exists; + // otherwise dashboard turns would redirect subsequent channel replies. if ( originatingChannel === INTERNAL_MESSAGE_CHANNEL && - !hasEstablishedExternalRouteForTo && + !establishedExternalChannel && (isMainSessionKey(params.sessionKey) || isDirectSessionKey(params.sessionKey)) ) { - return params.originatingToRaw || params.toRaw; - } - // When the turn originates from an internal/non-deliverable source, do not - // replace an established external destination with internal routing ids - // (e.g., session/webchat ids). - if (!isExternalRoutingChannel(originatingChannel)) { - const hasExternalFallback = - isExternalRoutingChannel(persistedChannel) || isExternalRoutingChannel(sessionKeyChannelHint); - if (hasExternalFallback && params.persistedLastTo) { - return params.persistedLastTo; - } + return { + channel: params.originatingChannelRaw, + to: params.originatingToRaw || params.toRaw, + }; } - return params.originatingToRaw || params.toRaw || params.persistedLastTo; + // Preserve channel and destination together so inter-session/internal turns + // cannot redirect an established external conversation to the dashboard. + const preserveExternalRoute = Boolean( + establishedExternalChannel && + (params.isInterSession || !isExternalRoutingChannel(originatingChannel)), + ); + return { + channel: preserveExternalRoute + ? params.isInterSession + ? (persistedChannel ?? establishedExternalChannel) + : establishedExternalChannel + : params.originatingChannelRaw || params.persistedLastChannel, + to: + (preserveExternalRoute && params.persistedLastTo) || + params.originatingToRaw || + params.toRaw || + params.persistedLastTo, + }; } export function maybeRetireLegacyMainDeliveryRoute(params: { diff --git a/src/auto-reply/reply/session.ts b/src/auto-reply/reply/session.ts index 4106a917d6e6..7cabd3d83382 100644 --- a/src/auto-reply/reply/session.ts +++ b/src/auto-reply/reply/session.ts @@ -113,8 +113,7 @@ import { isResetAuthorizedForContext } from "./reset-authorization.js"; import { resolveRuntimePolicySessionKey } from "./runtime-policy-session-key.js"; import { maybeRetireLegacyMainDeliveryRoute, - resolveLastChannelRaw, - resolveLastToRaw, + resolveSessionDeliveryRoute, } from "./session-delivery.js"; import { createReplySessionEntryHandle, @@ -824,17 +823,9 @@ async function initSessionStateAttemptLocked( const baseDeliveryContext = deliveryContextFromSession(baseEntry); const baseDeliveryRoute = sessionDeliveryRoute(baseEntry); const baseDeliveryOrigin = sessionDeliveryOrigin(baseEntry); - const lastChannelRaw = isSystemEvent - ? baseDeliveryContext?.channel - : resolveLastChannelRaw({ - originatingChannelRaw, - persistedLastChannel: baseDeliveryContext?.channel, - sessionKey, - isInterSession, - }); - const lastToRaw = isSystemEvent - ? baseDeliveryContext?.to - : resolveLastToRaw({ + const deliveryRoute = isSystemEvent + ? { channel: baseDeliveryContext?.channel, to: baseDeliveryContext?.to } + : resolveSessionDeliveryRoute({ originatingChannelRaw, originatingToRaw: ctx.OriginatingTo, toRaw: ctx.To, @@ -843,6 +834,7 @@ async function initSessionStateAttemptLocked( sessionKey, isInterSession, }); + const { channel: lastChannelRaw, to: lastToRaw } = deliveryRoute; const lastAccountIdRaw = isSystemEvent ? baseDeliveryContext?.accountId : resolveSessionDefaultAccountId({