mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 04:15:48 -06:00
refactor(auto-reply): unify session delivery route resolution (#129124)
This commit is contained in:
committed by
GitHub
parent
e9f78c9cb9
commit
59f566547b
@@ -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" });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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({
|
||||
|
||||
Reference in New Issue
Block a user