Skip to content

Commit 87dbe24

Browse files
samzongsteipete
samzong
authored and
steipete
committed
fix(sessions): prefer external delivery context
Signed-off-by: samzong <[email protected]>
1 parent 6e25112 commit 87dbe24

4 files changed

Lines changed: 319 additions & 6 deletions

File tree

‎src/agents/tools/sessions.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,43 @@ describe("resolveAnnounceTarget", () => {
408408
});
409409
});
410410

411+
it("hydrates announce delivery from explicit external context over stale webchat session fields", async () => {
412+
callGatewayMock.mockResolvedValueOnce({
413+
sessions: [
414+
{
415+
key: "agent:main:feishu:direct:ou_user",
416+
channel: "webchat",
417+
lastChannel: "webchat",
418+
lastTo: "session:dashboard",
419+
route: {
420+
channel: "webchat",
421+
target: { to: "session:dashboard" },
422+
},
423+
deliveryContext: {
424+
channel: "feishu",
425+
to: "user:ou_user",
426+
},
427+
origin: {
428+
provider: "feishu",
429+
accountId: "work",
430+
threadId: "thread-77",
431+
},
432+
},
433+
],
434+
});
435+
436+
const target = await resolveAnnounceTarget({
437+
sessionKey: "agent:main:feishu:direct:ou_user",
438+
displayKey: "agent:main:feishu:direct:ou_user",
439+
});
440+
expect(target).toEqual({
441+
channel: "feishu",
442+
to: "user:ou_user",
443+
accountId: "work",
444+
threadId: "thread-77",
445+
});
446+
});
447+
411448
it("preserves threaded Slack session keys when sessions.list lacks stored thread metadata", async () => {
412449
callGatewayMock.mockResolvedValueOnce({
413450
sessions: [

‎src/gateway/server.sessions-send.test.ts‎

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
import fs from "node:fs/promises";
22
import os from "node:os";
33
import path from "node:path";
4-
import { afterAll, beforeAll, beforeEach, describe, expect, it, type Mock } from "vitest";
4+
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi, type Mock } from "vitest";
5+
import { testing as agentStepTesting } from "../agents/tools/agent-step.js";
6+
import { runSessionsSendA2AFlow } from "../agents/tools/sessions-send-tool.a2a.js";
57
import { resolveSessionTranscriptPath } from "../config/sessions.js";
68
import type { OpenClawConfig } from "../config/types.openclaw.js";
79
import { emitAgentEvent } from "../infra/agent-events.js";
10+
import { createOutboundTestPlugin, createTestRegistry } from "../test-utils/channel-plugins.js";
811
import { captureEnv } from "../test-utils/env.js";
912
import {
1013
agentCommand,
1114
getFreePort,
1215
installGatewayTestHooks,
1316
startGatewayServer,
17+
setTestPluginRegistry,
1418
testState,
1519
writeSessionStore,
1620
} from "./test-helpers.js";
@@ -171,6 +175,114 @@ describe("sessions_send gateway loopback", () => {
171175
expect(firstCall?.inputProvenance?.kind).toBe("inter_session");
172176
expect(firstCall?.inputProvenance?.sourceTool).toBe("sessions_send");
173177
});
178+
179+
it(
180+
"announces through gateway send using external deliveryContext over stale webchat session fields",
181+
{ timeout: SESSION_SEND_E2E_TIMEOUT_MS },
182+
async () => {
183+
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sessions-send-route-"));
184+
const sendCalls: Array<{
185+
to?: string;
186+
text?: string;
187+
accountId?: string | null;
188+
threadId?: string | number | null;
189+
}> = [];
190+
setTestPluginRegistry(
191+
createTestRegistry([
192+
{
193+
pluginId: "whatsapp",
194+
source: "test",
195+
plugin: createOutboundTestPlugin({
196+
id: "whatsapp",
197+
label: "WhatsApp",
198+
outbound: {
199+
deliveryMode: "direct",
200+
resolveTarget: ({ to }) => {
201+
const target = to?.trim();
202+
return target
203+
? { ok: true, to: target }
204+
: { ok: false, error: new Error("missing target") };
205+
},
206+
sendText: async (ctx) => {
207+
sendCalls.push({
208+
to: ctx.to,
209+
text: ctx.text,
210+
accountId: ctx.accountId,
211+
threadId: ctx.threadId,
212+
});
213+
return { channel: "whatsapp", messageId: "wa-proof-msg" };
214+
},
215+
},
216+
messaging: {
217+
normalizeTarget: (raw) => raw,
218+
},
219+
}),
220+
},
221+
]),
222+
);
223+
224+
testState.sessionStorePath = path.join(dir, "sessions.json");
225+
try {
226+
await writeSessionStore({
227+
entries: {
228+
"agent:main:whatsapp:direct:peer-1": {
229+
sessionId: "sess-whatsapp-peer",
230+
updatedAt: Date.now(),
231+
channel: "webchat",
232+
lastChannel: "webchat",
233+
lastTo: "session:dashboard",
234+
route: {
235+
channel: "webchat",
236+
target: { to: "session:dashboard" },
237+
},
238+
deliveryContext: {
239+
channel: "whatsapp",
240+
to: "peer-1",
241+
},
242+
origin: {
243+
provider: "whatsapp",
244+
accountId: "work",
245+
threadId: "thread-77",
246+
},
247+
},
248+
},
249+
});
250+
251+
agentStepTesting.setDepsForTest({
252+
agentCommandFromIngress: async () => ({
253+
payloads: [{ text: "announce through channel", mediaUrl: null }],
254+
meta: { durationMs: 1 },
255+
}),
256+
});
257+
258+
await runSessionsSendA2AFlow({
259+
targetSessionKey: "agent:main:whatsapp:direct:peer-1",
260+
displayKey: "agent:main:whatsapp:direct:peer-1",
261+
message: "ping",
262+
announceTimeoutMs: 5_000,
263+
maxPingPongTurns: 0,
264+
roundOneReply: "target response",
265+
});
266+
267+
await vi.waitFor(
268+
() =>
269+
expect(sendCalls).toEqual([
270+
{
271+
to: "peer-1",
272+
text: "announce through channel",
273+
accountId: "work",
274+
threadId: "thread-77",
275+
},
276+
]),
277+
{ timeout: 5_000 },
278+
);
279+
} finally {
280+
agentStepTesting.setDepsForTest();
281+
testState.sessionStorePath = undefined;
282+
await fs.rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 });
283+
}
284+
},
285+
);
174286
});
175287

176288
describe("sessions_send label lookup", () => {

‎src/utils/delivery-context.shared.ts‎

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@ import {
88
} from "../plugin-sdk/channel-route.js";
99
import { normalizeAccountId } from "./account-id.js";
1010
import type { DeliveryContext, DeliveryContextSessionSource } from "./delivery-context.types.js";
11+
import {
12+
INTERNAL_MESSAGE_CHANNEL,
13+
isInternalNonDeliveryChannel,
14+
} from "./message-channel-constants.js";
1115
import { normalizeMessageChannel } from "./message-channel-core.js";
16+
import { isDeliverableMessageChannel } from "./message-channel-normalize.js";
1217
export type { DeliveryContext, DeliveryContextSessionSource } from "./delivery-context.types.js";
1318

1419
export function normalizeDeliveryContext(context?: DeliveryContext): DeliveryContext | undefined {
@@ -93,6 +98,30 @@ function mergeRouteMetadataWithDeliveryContext(
9398
});
9499
}
95100

101+
function isInternalRouteContext(context?: DeliveryContext): boolean {
102+
const channel = context?.channel;
103+
return Boolean(
104+
channel && (channel === INTERNAL_MESSAGE_CHANNEL || isInternalNonDeliveryChannel(channel)),
105+
);
106+
}
107+
108+
function hasExternalDeliveryTarget(context?: DeliveryContext): boolean {
109+
const channel = normalizeMessageChannel(context?.channel);
110+
return Boolean(channel && isDeliverableMessageChannel(channel) && context?.to);
111+
}
112+
113+
function mergeExternalDeliveryContextOverInternalRoute(
114+
deliveryContext?: DeliveryContext,
115+
internalContext?: DeliveryContext,
116+
): DeliveryContext | undefined {
117+
return normalizeDeliveryContext({
118+
channel: deliveryContext?.channel,
119+
to: deliveryContext?.to,
120+
accountId: deliveryContext?.accountId ?? internalContext?.accountId,
121+
threadId: deliveryContext?.threadId ?? internalContext?.threadId,
122+
});
123+
}
124+
96125
export function normalizeSessionDeliveryFields(source?: DeliveryContextSessionSource): {
97126
route?: ChannelRouteRef;
98127
deliveryContext?: DeliveryContext;
@@ -120,10 +149,17 @@ export function normalizeSessionDeliveryFields(source?: DeliveryContextSessionSo
120149
accountId: source.lastAccountId,
121150
threadId: source.lastThreadId,
122151
});
123-
const merged = mergeDeliveryContext(
124-
routeContext,
125-
mergeDeliveryContext(legacyContext, normalizeDeliveryContext(source.deliveryContext)),
126-
);
152+
const deliveryContext = normalizeDeliveryContext(source.deliveryContext);
153+
const sessionContext =
154+
isInternalRouteContext(legacyContext) && hasExternalDeliveryTarget(deliveryContext)
155+
? mergeExternalDeliveryContextOverInternalRoute(deliveryContext, legacyContext)
156+
: mergeDeliveryContext(legacyContext, deliveryContext);
157+
const routeInternalContext = mergeDeliveryContext(routeContext, legacyContext);
158+
const routeIsInternalFallback =
159+
isInternalRouteContext(routeContext) && hasExternalDeliveryTarget(deliveryContext);
160+
const merged = routeIsInternalFallback
161+
? mergeExternalDeliveryContextOverInternalRoute(deliveryContext, routeInternalContext)
162+
: mergeDeliveryContext(routeContext, sessionContext);
127163

128164
if (!merged) {
129165
return {
@@ -137,7 +173,10 @@ export function normalizeSessionDeliveryFields(source?: DeliveryContextSessionSo
137173
}
138174

139175
return {
140-
route: mergeRouteMetadataWithDeliveryContext(normalizedRoute, merged),
176+
route: mergeRouteMetadataWithDeliveryContext(
177+
routeIsInternalFallback ? undefined : normalizedRoute,
178+
merged,
179+
),
141180
deliveryContext: merged,
142181
lastChannel: merged.channel,
143182
lastTo: merged.to,

‎src/utils/delivery-context.test.ts‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,131 @@ describe("delivery context helpers", () => {
261261
});
262262
});
263263

264+
it("prefers explicit external delivery context over stale webchat legacy fields", () => {
265+
expect(
266+
deliveryContextFromSession({
267+
channel: "webchat",
268+
deliveryContext: {
269+
channel: "room-chat",
270+
to: " peer-1 ",
271+
accountId: " acct-1 ",
272+
threadId: " thread-1 ",
273+
},
274+
}),
275+
).toEqual({
276+
channel: "room-chat",
277+
to: "peer-1",
278+
accountId: "acct-1",
279+
threadId: "thread-1",
280+
});
281+
282+
expect(
283+
deliveryContextFromSession({
284+
channel: "webchat",
285+
lastChannel: "webchat",
286+
lastTo: "session:dashboard",
287+
lastAccountId: "work",
288+
lastThreadId: "thread-2",
289+
deliveryContext: {
290+
channel: "room-chat",
291+
to: "peer-2",
292+
},
293+
}),
294+
).toEqual({
295+
channel: "room-chat",
296+
to: "peer-2",
297+
accountId: "work",
298+
threadId: "thread-2",
299+
});
300+
301+
expect(
302+
deliveryContextFromSession({
303+
lastChannel: "heartbeat",
304+
lastTo: "heartbeat",
305+
deliveryContext: {
306+
channel: "telegram",
307+
to: "-100123",
308+
},
309+
}),
310+
).toEqual({
311+
channel: "telegram",
312+
to: "-100123",
313+
accountId: undefined,
314+
});
315+
316+
const routeNormalized = normalizeSessionDeliveryFields({
317+
route: {
318+
channel: "webchat",
319+
accountId: "work",
320+
target: { to: "session:dashboard" },
321+
thread: { id: "thread-route" },
322+
},
323+
deliveryContext: {
324+
channel: "room-chat",
325+
to: "peer-route",
326+
},
327+
});
328+
expect(routeNormalized.deliveryContext).toEqual({
329+
channel: "room-chat",
330+
to: "peer-route",
331+
accountId: "work",
332+
threadId: "thread-route",
333+
});
334+
expect(routeNormalized.route).toEqual({
335+
channel: "room-chat",
336+
accountId: "work",
337+
target: { to: "peer-route" },
338+
thread: { id: "thread-route" },
339+
});
340+
});
341+
342+
it("does not promote tool-only context over internal session delivery", () => {
343+
const normalized = normalizeSessionDeliveryFields({
344+
route: {
345+
channel: "webchat",
346+
accountId: "work",
347+
target: { to: "session:dashboard" },
348+
},
349+
deliveryContext: {
350+
channel: "sessions_send",
351+
to: "session:handoff",
352+
},
353+
});
354+
355+
expect(normalized.deliveryContext).toEqual({
356+
channel: "webchat",
357+
to: "session:dashboard",
358+
accountId: "work",
359+
});
360+
expect(normalized.route).toEqual({
361+
channel: "webchat",
362+
accountId: "work",
363+
target: { to: "session:dashboard" },
364+
});
365+
366+
const staleLegacyExternal = normalizeSessionDeliveryFields({
367+
route: {
368+
channel: "webchat",
369+
accountId: "work",
370+
target: { to: "session:dashboard" },
371+
},
372+
lastChannel: "room-chat",
373+
lastTo: "peer-old",
374+
lastAccountId: "old-workspace",
375+
});
376+
377+
expect(staleLegacyExternal.deliveryContext).toEqual({
378+
channel: "webchat",
379+
to: "session:dashboard",
380+
accountId: "work",
381+
});
382+
expect(staleLegacyExternal.route).toEqual({
383+
channel: "webchat",
384+
accountId: "work",
385+
target: { to: "session:dashboard" },
386+
});
387+
});
388+
264389
it("normalizes delivery fields, mirrors session fields, and avoids cross-channel carryover", () => {
265390
const normalized = normalizeSessionDeliveryFields({
266391
deliveryContext: {

0 commit comments

Comments
 (0)