Skip to content

Commit 89e1a04

Browse files
ooiuuiiooiuuii
authored
Merge f838b0e into 538d36e 
2 parents 538d36e + f838b0e commit 89e1a04

4 files changed

Lines changed: 314 additions & 11 deletions

File tree

‎src/agents/embedded-agent-runner/run/payloads.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,27 @@ describe("buildEmbeddedRunPayloads tool-error warnings", () => {
225225
expectSinglePayloadText(payloads, "The schema export is fixed.");
226226
});
227227

228+
it("suppresses private finals after delivered current-source message-tool replies", () => {
229+
const payloads = buildPayloads({
230+
assistantTexts: ["I already sent the answer through the message tool."],
231+
didSendViaMessagingTool: true,
232+
messagingToolSourceReplyPayloads: [
233+
{
234+
text: "The requested status check completed successfully.",
235+
},
236+
],
237+
sourceReplyDeliveryMode: "automatic",
238+
sessionKey: "agent:main:test:direct:source",
239+
agentId: "main",
240+
runId: "run-current-source-reply",
241+
});
242+
243+
// Automatic-mode message-tool sends have already been delivered by the
244+
// message tool path; the final payload builder should only suppress the
245+
// follow-up private bookkeeping text, not resend the source reply.
246+
expect(payloads).toEqual([]);
247+
});
248+
228249
it("turns internal message-tool source replies into suppression-safe final payloads", () => {
229250
// message_tool_only source replies are already delivered internally but
230251
// still need mirror metadata so transcript/persistence can record them.

‎src/agents/embedded-agent-runner/run/payloads.ts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,25 @@ export function buildEmbeddedRunPayloads(params: {
272272
};
273273
}> = [];
274274

275+
const deliveredSourceReplyPayloads = params.messagingToolSourceReplyPayloads ?? [];
276+
const sourceReplyPayloadHasContent = (payload: MessagingToolSourceReplyPayload): boolean => {
277+
const text = normalizeOptionalString(payload.text) ?? "";
278+
const media = Array.from(
279+
new Set([...(payload.mediaUrl ? [payload.mediaUrl] : []), ...(payload.mediaUrls ?? [])]),
280+
).filter((value) => value.trim().length > 0);
281+
return Boolean(
282+
text ||
283+
media.length > 0 ||
284+
payload.presentation ||
285+
payload.interactive ||
286+
payload.channelData,
287+
);
288+
};
289+
const hasDeliveredSourceReplyPayload = deliveredSourceReplyPayloads.some(
290+
sourceReplyPayloadHasContent,
291+
);
275292
const sourceReplyPayloads =
276-
params.sourceReplyDeliveryMode === "message_tool_only"
277-
? (params.messagingToolSourceReplyPayloads ?? [])
278-
: [];
293+
params.sourceReplyDeliveryMode === "message_tool_only" ? deliveredSourceReplyPayloads : [];
279294
const sourceReplyStartIndex = replyItems.length;
280295
sourceReplyPayloads.forEach((payload, index) => {
281296
const text = normalizeOptionalString(payload.text) ?? "";
@@ -312,7 +327,9 @@ export function buildEmbeddedRunPayloads(params: {
312327

313328
const useMarkdown = params.toolResultFormat === "markdown";
314329
const suppressAssistantArtifacts =
315-
params.didSendDeterministicApprovalPrompt === true || hasSourceReplyPayload;
330+
params.didSendDeterministicApprovalPrompt === true ||
331+
hasSourceReplyPayload ||
332+
hasDeliveredSourceReplyPayload;
316333
const nonEmptyAssistantTexts = params.assistantTexts.filter((text) => text.trim().length > 0);
317334
const currentAssistant = params.currentAssistant ?? undefined;
318335
const assistantForPayload =

‎src/infra/outbound/message-action-runner.core-send.test.ts‎

Lines changed: 121 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,14 @@ const slackConfig = {
3737
},
3838
} as OpenClawConfig;
3939

40-
function registerSlackTextPlugin() {
41-
const sendText = vi.fn().mockResolvedValue({
40+
function registerSlackTextPlugin(
41+
result: Record<string, unknown> = {
4242
channel: "slack",
4343
messageId: "m1",
4444
chatId: "C123",
45-
});
45+
},
46+
) {
47+
const sendText = vi.fn().mockResolvedValue(result);
4648
setActivePluginRegistry(
4749
createTestRegistry([
4850
{
@@ -237,6 +239,122 @@ describe("runMessageAction core send routing", () => {
237239
expect(payload.dryRun).toBe(true);
238240
});
239241

242+
it("marks successful current-session sends as delivered source replies", async () => {
243+
const sendText = registerSlackTextPlugin();
244+
245+
const result = await runMessageAction({
246+
cfg: slackConfig,
247+
action: "send",
248+
params: {
249+
message: "visible source reply",
250+
},
251+
toolContext: {
252+
currentChannelProvider: "slack",
253+
currentChannelId: "channel:C123",
254+
},
255+
sessionKey: "agent:main:slack:channel:c123",
256+
dryRun: false,
257+
});
258+
259+
expect(result.kind).toBe("send");
260+
expect(sendText).toHaveBeenCalledOnce();
261+
expect(result.payload).toMatchObject({
262+
deliveryStatus: "sent",
263+
sourceReplySink: "internal-ui",
264+
sourceReply: {
265+
text: "visible source reply",
266+
},
267+
message: "visible source reply",
268+
});
269+
});
270+
271+
it("does not mark sends to another session as current-source replies", async () => {
272+
const sendText = registerSlackTextPlugin();
273+
274+
const result = await runMessageAction({
275+
cfg: slackConfig,
276+
action: "send",
277+
params: {
278+
target: "channel:C999",
279+
message: "send elsewhere",
280+
},
281+
toolContext: {
282+
currentChannelProvider: "slack",
283+
currentChannelId: "channel:C123",
284+
},
285+
sessionKey: "agent:main:slack:channel:c123",
286+
dryRun: false,
287+
});
288+
289+
expect(result.kind).toBe("send");
290+
expect(sendText).toHaveBeenCalledOnce();
291+
expect(result.payload).not.toMatchObject({
292+
sourceReplySink: "internal-ui",
293+
});
294+
});
295+
296+
it("does not mark ambiguous current-session sends as delivered source replies", async () => {
297+
const sendText = registerSlackTextPlugin({
298+
channel: "slack",
299+
chatId: "C123",
300+
});
301+
302+
const result = await runMessageAction({
303+
cfg: slackConfig,
304+
action: "send",
305+
params: {
306+
message: "final should remain eligible when delivery is ambiguous",
307+
},
308+
toolContext: {
309+
currentChannelProvider: "slack",
310+
currentChannelId: "channel:C123",
311+
},
312+
sessionKey: "agent:main:slack:channel:c123",
313+
dryRun: false,
314+
});
315+
316+
expect(result.kind).toBe("send");
317+
expect(sendText).toHaveBeenCalledOnce();
318+
expect(result.payload).not.toMatchObject({
319+
deliveryStatus: "sent",
320+
sourceReplySink: "internal-ui",
321+
});
322+
});
323+
324+
it("does not mark failed current-session sends as delivered source replies", async () => {
325+
const sendText = registerSlackTextPlugin({
326+
channel: "slack",
327+
chatId: "C123",
328+
deliveryStatus: "failed",
329+
error: "send failed",
330+
});
331+
332+
const result = await runMessageAction({
333+
cfg: slackConfig,
334+
action: "send",
335+
params: {
336+
message: "final should remain eligible after failed delivery",
337+
},
338+
toolContext: {
339+
currentChannelProvider: "slack",
340+
currentChannelId: "channel:C123",
341+
},
342+
sessionKey: "agent:main:slack:channel:c123",
343+
dryRun: false,
344+
});
345+
346+
expect(result.kind).toBe("send");
347+
expect(sendText).toHaveBeenCalledOnce();
348+
expect(result.payload).toMatchObject({
349+
result: {
350+
deliveryStatus: "failed",
351+
},
352+
});
353+
expect(result.payload).not.toMatchObject({
354+
sourceReplySink: "internal-ui",
355+
});
356+
});
357+
240358
it("uses best-effort delivery for implicit message-tool-only source replies", async () => {
241359
const sendText = registerSlackTextPlugin();
242360

‎src/infra/outbound/message-action-runner.ts‎

Lines changed: 151 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ import {
8787
shouldApplyCrossContextMarker,
8888
} from "./outbound-policy.js";
8989
import { executePollAction, executeSendAction } from "./outbound-send-service.js";
90-
import { ensureOutboundSessionEntry, resolveOutboundSessionRoute } from "./outbound-session.js";
90+
import {
91+
ensureOutboundSessionEntry,
92+
resolveOutboundSessionRoute,
93+
type OutboundSessionRoute,
94+
} from "./outbound-session.js";
9195
import { normalizeTargetForProvider } from "./target-normalization.js";
9296
import { resolveChannelTarget, type ResolvedMessagingTarget } from "./target-resolver.js";
9397
import { extractToolPayload } from "./tool-payload.js";
@@ -1073,6 +1077,116 @@ async function buildSendPayloadParts(params: {
10731077
};
10741078
}
10751079

1080+
function isCurrentSourceOutboundRoute(params: {
1081+
input: RunMessageActionParams;
1082+
actionParams: Record<string, unknown>;
1083+
outboundRoute: OutboundSessionRoute | null;
1084+
dryRun: boolean;
1085+
}): boolean {
1086+
if (params.dryRun) {
1087+
return false;
1088+
}
1089+
const currentSessionKey = normalizeOptionalLowercaseString(params.input.sessionKey);
1090+
const outboundSessionKey = normalizeOptionalLowercaseString(params.outboundRoute?.sessionKey);
1091+
if (currentSessionKey && outboundSessionKey && currentSessionKey === outboundSessionKey) {
1092+
return true;
1093+
}
1094+
return (
1095+
!hasExplicitNonCurrentChannelParam(params.input, params.actionParams) &&
1096+
isCurrentSourceTargetParam(params.input, params.actionParams)
1097+
);
1098+
}
1099+
1100+
function collectDeliveryEvidencePayloads(
1101+
payloadRecord: Record<string, unknown>,
1102+
): Record<string, unknown>[] {
1103+
const nested = payloadRecord.result;
1104+
if (nested && typeof nested === "object" && !Array.isArray(nested)) {
1105+
return [payloadRecord, nested as Record<string, unknown>];
1106+
}
1107+
return [payloadRecord];
1108+
}
1109+
1110+
function payloadHasPositiveDeliveryEvidence(payloadRecord: Record<string, unknown>): boolean {
1111+
let hasDeliveredStatus = false;
1112+
let hasMessageId = false;
1113+
for (const candidate of collectDeliveryEvidencePayloads(payloadRecord)) {
1114+
const deliveryStatus = normalizeOptionalString(candidate.deliveryStatus)?.toLowerCase();
1115+
const status = normalizeOptionalString(candidate.status)?.toLowerCase();
1116+
if (
1117+
deliveryStatus === "failed" ||
1118+
deliveryStatus === "error" ||
1119+
deliveryStatus === "undelivered" ||
1120+
status === "failed" ||
1121+
status === "error" ||
1122+
status === "undelivered" ||
1123+
candidate.ok === false
1124+
) {
1125+
return false;
1126+
}
1127+
hasDeliveredStatus ||= deliveryStatus === "sent" || deliveryStatus === "delivered";
1128+
hasDeliveredStatus ||= status === "sent" || status === "delivered";
1129+
hasMessageId ||= Boolean(
1130+
normalizeOptionalString(candidate.messageId) ?? normalizeOptionalString(candidate.message_id),
1131+
);
1132+
}
1133+
return hasDeliveredStatus || hasMessageId;
1134+
}
1135+
1136+
function annotatePayloadForCurrentSourceReply(params: {
1137+
payload: unknown;
1138+
sendPayload: SendPayloadParts;
1139+
input: RunMessageActionParams;
1140+
actionParams: Record<string, unknown>;
1141+
outboundRoute: OutboundSessionRoute | null;
1142+
dryRun: boolean;
1143+
}): unknown {
1144+
if (!isCurrentSourceOutboundRoute(params)) {
1145+
return params.payload;
1146+
}
1147+
const payloadRecord: Record<string, unknown> =
1148+
params.payload && typeof params.payload === "object" && !Array.isArray(params.payload)
1149+
? { ...(params.payload as Record<string, unknown>) }
1150+
: { result: params.payload };
1151+
if (!payloadHasPositiveDeliveryEvidence(payloadRecord)) {
1152+
return params.payload;
1153+
}
1154+
return {
1155+
...payloadRecord,
1156+
deliveryStatus: "sent",
1157+
sourceReplySink: "internal-ui" as const,
1158+
...(params.input.sourceReplyDeliveryMode
1159+
? { sourceReplyDeliveryMode: params.input.sourceReplyDeliveryMode }
1160+
: {}),
1161+
sourceReply: params.sendPayload.payload,
1162+
...(params.sendPayload.message ? { message: params.sendPayload.message } : {}),
1163+
...(params.sendPayload.mediaUrl ? { mediaUrl: params.sendPayload.mediaUrl } : {}),
1164+
...(params.sendPayload.mediaUrls?.length ? { mediaUrls: params.sendPayload.mediaUrls } : {}),
1165+
};
1166+
}
1167+
1168+
function annotateToolResultForCurrentSourceReply(
1169+
toolResult: AgentToolResult<unknown> | undefined,
1170+
params: {
1171+
sendPayload: SendPayloadParts;
1172+
input: RunMessageActionParams;
1173+
actionParams: Record<string, unknown>;
1174+
outboundRoute: OutboundSessionRoute | null;
1175+
dryRun: boolean;
1176+
},
1177+
): AgentToolResult<unknown> | undefined {
1178+
if (!toolResult || !isCurrentSourceOutboundRoute(params)) {
1179+
return toolResult;
1180+
}
1181+
return {
1182+
...toolResult,
1183+
details: annotatePayloadForCurrentSourceReply({
1184+
payload: toolResult.details,
1185+
...params,
1186+
}),
1187+
};
1188+
}
1189+
10761190
async function handleSendAction(ctx: ResolvedActionContext): Promise<MessageActionRunResult> {
10771191
const {
10781192
cfg,
@@ -1171,7 +1285,27 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise<MessageActi
11711285
}),
11721286
});
11731287
if (gatewayPluginAction) {
1174-
return gatewayPluginAction;
1288+
if (gatewayPluginAction.kind !== "send") {
1289+
return gatewayPluginAction;
1290+
}
1291+
return {
1292+
...gatewayPluginAction,
1293+
payload: annotatePayloadForCurrentSourceReply({
1294+
payload: gatewayPluginAction.payload,
1295+
sendPayload,
1296+
input,
1297+
actionParams: params,
1298+
outboundRoute,
1299+
dryRun,
1300+
}),
1301+
toolResult: annotateToolResultForCurrentSourceReply(gatewayPluginAction.toolResult, {
1302+
sendPayload,
1303+
input,
1304+
actionParams: params,
1305+
outboundRoute,
1306+
dryRun,
1307+
}),
1308+
};
11751309
}
11761310

11771311
const send = await executeSendAction({
@@ -1230,8 +1364,21 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise<MessageActi
12301364
action,
12311365
to,
12321366
handledBy: send.handledBy,
1233-
payload: send.payload,
1234-
toolResult: send.toolResult,
1367+
payload: annotatePayloadForCurrentSourceReply({
1368+
payload: send.payload,
1369+
sendPayload,
1370+
input,
1371+
actionParams: params,
1372+
outboundRoute,
1373+
dryRun,
1374+
}),
1375+
toolResult: annotateToolResultForCurrentSourceReply(send.toolResult, {
1376+
sendPayload,
1377+
input,
1378+
actionParams: params,
1379+
outboundRoute,
1380+
dryRun,
1381+
}),
12351382
sendResult: send.sendResult,
12361383
dryRun,
12371384
};

0 commit comments

Comments
 (0)