Skip to content

Commit 0ea0807

Browse files
vincentkocvincentkoc
committed
fix(agents): preserve CLI message delivery evidence
1 parent 04d8a96 commit 0ea0807

53 files changed

Lines changed: 5545 additions & 667 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎extensions/slack/src/channel-actions.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@ type SlackActionInvoke = (
1515

1616
let slackActionRuntimePromise: Promise<typeof import("./action-runtime.runtime.js")> | undefined;
1717

18+
const SLACK_TOOL_DELIVERY_ACTIONS = new Set([
19+
"deleteMessage",
20+
"editMessage",
21+
"pinMessage",
22+
"react",
23+
"sendMessage",
24+
"unpinMessage",
25+
"uploadFile",
26+
]);
27+
1828
async function loadSlackActionRuntime() {
1929
slackActionRuntimePromise ??= import("./action-runtime.runtime.js");
2030
return await slackActionRuntimePromise;
@@ -42,6 +52,8 @@ export function createSlackActions(
4252
return {
4353
describeMessageTool: describeSlackMessageTool,
4454
extractToolSend: ({ args }) => extractSlackToolSend(args),
55+
isToolDeliveryAction: ({ args }) =>
56+
typeof args.action === "string" && SLACK_TOOL_DELIVERY_ACTIONS.has(args.action),
4557
prepareSendPayload: ({ ctx, payload }) => (ctx.action === "send" ? payload : null),
4658
handleAction: async (ctx) => {
4759
return await handleSlackMessageAction({

‎extensions/slack/src/message-tools.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Slack tests cover message tools plugin behavior.
22
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
33
import { describe, expect, it } from "vitest";
4+
import { createSlackActions } from "./channel-actions.js";
45
import { listSlackMessageActions } from "./message-actions.js";
56
import { describeSlackMessageTool } from "./message-tool-api.js";
67

@@ -24,6 +25,16 @@ function requireSchemaProperty(
2425
}
2526

2627
describe("Slack message tools", () => {
28+
it("classifies provider-native mutation actions", () => {
29+
const actions = createSlackActions("slack");
30+
for (const action of ["sendMessage", "editMessage", "deleteMessage", "pinMessage"]) {
31+
expect(actions.isToolDeliveryAction?.({ args: { action } })).toBe(true);
32+
}
33+
for (const action of ["readMessages", "listPins", "downloadFile"]) {
34+
expect(actions.isToolDeliveryAction?.({ args: { action } })).toBe(false);
35+
}
36+
});
37+
2738
it("describes configured Slack message actions without loading channel runtime", () => {
2839
const discovery = describeSlackMessageTool({
2940
cfg: {

‎extensions/telegram/src/channel-actions.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ describe("telegramMessageActions", () => {
2727
}
2828
});
2929

30+
it("classifies provider-native mutation actions", () => {
31+
for (const action of ["sendMessage", "editMessage", "deleteMessage", "react", "topic-edit"]) {
32+
expect(telegramMessageActions.isToolDeliveryAction?.({ args: { action } })).toBe(true);
33+
}
34+
for (const action of ["searchSticker", "stickerCacheStats"]) {
35+
expect(telegramMessageActions.isToolDeliveryAction?.({ args: { action } })).toBe(false);
36+
}
37+
});
38+
3039
it("allows interactive-only sends", async () => {
3140
await telegramMessageActions.handleAction!({
3241
action: "send",

‎extensions/telegram/src/channel-actions.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,23 @@ const TELEGRAM_MESSAGE_ACTION_MAP = {
5050
"topic-edit": "editForumTopic",
5151
} as const satisfies Partial<Record<ChannelMessageActionName, string>>;
5252

53+
const TELEGRAM_TOOL_DELIVERY_ACTIONS = new Set([
54+
"createForumTopic",
55+
"delete",
56+
"deleteMessage",
57+
"edit",
58+
"editForumTopic",
59+
"editMessage",
60+
"poll",
61+
"react",
62+
"send",
63+
"sendMessage",
64+
"sendSticker",
65+
"sticker",
66+
"topic-create",
67+
"topic-edit",
68+
]);
69+
5370
function resolveTelegramMessageActionName(action: ChannelMessageActionName) {
5471
return TELEGRAM_MESSAGE_ACTION_MAP[action as keyof typeof TELEGRAM_MESSAGE_ACTION_MAP];
5572
}
@@ -181,6 +198,8 @@ export const telegramMessageActions: ChannelMessageActionAdapter = {
181198
extractToolSend: ({ args }) => {
182199
return extractToolSend(args, "sendMessage");
183200
},
201+
isToolDeliveryAction: ({ args }) =>
202+
typeof args.action === "string" && TELEGRAM_TOOL_DELIVERY_ACTIONS.has(args.action),
184203
handleAction: async ({
185204
action,
186205
params,

‎extensions/telegram/src/channel.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,10 @@ const telegramMessageActions: ChannelMessageActionAdapter = {
289289
getOptionalTelegramRuntime()?.channel?.telegram?.messageActions?.extractToolSend?.(ctx) ??
290290
telegramMessageActionsImpl.extractToolSend?.(ctx) ??
291291
null,
292+
isToolDeliveryAction: (ctx) =>
293+
getOptionalTelegramRuntime()?.channel?.telegram?.messageActions?.isToolDeliveryAction?.(ctx) ??
294+
telegramMessageActionsImpl.isToolDeliveryAction?.(ctx) ??
295+
false,
292296
handleAction: async (ctx) => {
293297
const runtimeHandleAction =
294298
getOptionalTelegramRuntime()?.channel?.telegram?.messageActions?.handleAction;

‎src/agents/cli-output.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,26 @@ import {
55
extractCliErrorMessage,
66
parseCliJson,
77
parseCliJsonl,
8+
supportsCliJsonlToolEvents,
89
type CliToolUseStartDelta,
910
} from "./cli-output.js";
1011
import { createClaudeApiErrorFixture } from "./test-helpers/claude-api-error-fixture.js";
1112

13+
describe("supportsCliJsonlToolEvents", () => {
14+
it.each([
15+
["Claude provider", { command: "claude", output: "jsonl" as const }, "claude-cli", true],
16+
[
17+
"explicit Claude dialect",
18+
{ command: "custom", output: "jsonl" as const, jsonlDialect: "claude-stream-json" as const },
19+
"custom-cli",
20+
true,
21+
],
22+
["generic JSONL", { command: "custom", output: "jsonl" as const }, "custom-cli", false],
23+
])("%s: %s", (_name, backend, providerId, expected) => {
24+
expect(supportsCliJsonlToolEvents({ backend, providerId })).toBe(expected);
25+
});
26+
});
27+
1228
describe("parseCliJson", () => {
1329
it("recovers mixed-output Claude session metadata from embedded JSON objects", () => {
1430
const result = parseCliJson(

‎src/agents/cli-output.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import { normalizeStringEntries } from "@openclaw/normalization-core/string-norm
88
import type { CliBackendConfig } from "../config/types.js";
99
import { extractBalancedJsonFragments } from "../shared/balanced-json.js";
1010
import { isRecord } from "../utils.js";
11+
import type {
12+
MessagingToolSend,
13+
MessagingToolSourceReplyPayload,
14+
} from "./embedded-agent-messaging.types.js";
1115

1216
type CliUsage = {
1317
input?: number;
@@ -24,6 +28,12 @@ export type CliOutput = {
2428
sessionId?: string;
2529
usage?: CliUsage;
2630
finalPromptText?: string;
31+
didSendViaMessagingTool?: boolean;
32+
didDeliverSourceReplyViaMessageTool?: boolean;
33+
messagingToolSentTexts?: string[];
34+
messagingToolSentMediaUrls?: string[];
35+
messagingToolSentTargets?: MessagingToolSend[];
36+
messagingToolSourceReplyPayloads?: MessagingToolSourceReplyPayload[];
2737
};
2838

2939
/** Incremental assistant text emitted while parsing a streaming CLI response. */
@@ -53,7 +63,8 @@ function isClaudeCliProvider(providerId: string): boolean {
5363
return normalizeLowercaseStringOrEmpty(providerId) === "claude-cli";
5464
}
5565

56-
function usesClaudeStreamJsonDialect(params: {
66+
/** Returns whether JSONL output carries correlated Claude-style tool events. */
67+
export function supportsCliJsonlToolEvents(params: {
5768
backend: CliBackendConfig;
5869
providerId: string;
5970
}): boolean {
@@ -67,7 +78,7 @@ function isClaudeStreamJsonResult(params: {
6778
providerId: string;
6879
parsed: Record<string, unknown>;
6980
}): boolean {
70-
return usesClaudeStreamJsonDialect(params) && params.parsed.type === "result";
81+
return supportsCliJsonlToolEvents(params) && params.parsed.type === "result";
7182
}
7283

7384
function extractJsonObjectCandidates(raw: string): string[] {
@@ -368,7 +379,7 @@ function parseClaudeCliJsonlResult(params: {
368379
sessionId?: string;
369380
usage?: CliUsage;
370381
}): CliOutput | null {
371-
if (!usesClaudeStreamJsonDialect(params)) {
382+
if (!supportsCliJsonlToolEvents(params)) {
372383
return null;
373384
}
374385
if (
@@ -395,7 +406,7 @@ function parseClaudeCliStreamingDelta(params: {
395406
sessionId?: string;
396407
usage?: CliUsage;
397408
}): CliStreamingDelta | null {
398-
if (!usesClaudeStreamJsonDialect(params)) {
409+
if (!supportsCliJsonlToolEvents(params)) {
399410
return null;
400411
}
401412
if (params.parsed.type !== "stream_event" || !isRecord(params.parsed.event)) {
@@ -510,7 +521,7 @@ function dispatchClaudeCliStreamingToolEvent(params: {
510521
onToolUseStart?: (delta: CliToolUseStartDelta) => void;
511522
onToolResult?: (delta: CliToolResultDelta) => void;
512523
}): void {
513-
if (!usesClaudeStreamJsonDialect(params)) {
524+
if (!supportsCliJsonlToolEvents(params)) {
514525
return;
515526
}
516527
const tracker = params.tracker;
@@ -644,7 +655,7 @@ export function createCliJsonlStreamingParser(params: {
644655
// Classification is keyed on consumer presence so reclassified pre-tool text
645656
// always has a destination; a separate enable flag let it be dropped (#92092).
646657
const classifyClaudeCommentary =
647-
Boolean(params.onCommentaryText) && usesClaudeStreamJsonDialect(params);
658+
Boolean(params.onCommentaryText) && supportsCliJsonlToolEvents(params);
648659

649660
const flushPendingClaudeAssistantText = () => {
650661
if (!pendingClaudeText) {

‎src/agents/cli-runner.before-agent-reply-cron.test.ts‎

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/** Tests cron before_agent_reply gating at the CLI runner entrypoint. */
22
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
33
import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js";
4+
import type { CliOutput } from "./cli-output.js";
45
import { cliBackendLog } from "./cli-runner/log.js";
56

67
// vi.mock factories are hoisted above imports, so any references inside them
@@ -28,9 +29,9 @@ const {
2829
runBeforeAgentReplyMock: vi.fn<(event: unknown, ctx: unknown) => Promise<BeforeAgentReplyResult>>(
2930
async () => undefined,
3031
),
31-
executePreparedCliRunMock: vi.fn(async (_context: unknown, _cliSessionIdToUse?: string) => ({
32-
text: "",
33-
})),
32+
executePreparedCliRunMock: vi.fn<
33+
(_context: unknown, _cliSessionIdToUse?: string) => Promise<CliOutput>
34+
>(async () => ({ text: "" })),
3435
prepareCliRunContextMock: vi.fn(),
3536
closeClaudeLiveSessionForContextMock: vi.fn(),
3637
closeMcpLoopbackServerMock: vi.fn(),
@@ -262,6 +263,35 @@ describe("runCliAgent cron before_agent_reply seam", () => {
262263
expect(executePreparedCliRunMock).toHaveBeenCalledTimes(1);
263264
});
264265

266+
it("reports confirmed CLI messaging delivery evidence without leaking it to later invocations", async () => {
267+
executePreparedCliRunMock.mockResolvedValueOnce({
268+
text: "sent",
269+
didSendViaMessagingTool: true,
270+
messagingToolSentTargets: [
271+
{
272+
tool: "message",
273+
provider: "telegram",
274+
to: "chat123",
275+
},
276+
],
277+
});
278+
executePreparedCliRunMock.mockResolvedValueOnce({ text: "later" });
279+
280+
const firstResult = await runCliAgent(baseRunParams);
281+
expect(firstResult.didSendViaMessagingTool).toBe(true);
282+
expect(firstResult.messagingToolSentTargets).toEqual([
283+
expect.objectContaining({
284+
tool: "message",
285+
provider: "telegram",
286+
to: "chat123",
287+
}),
288+
]);
289+
290+
const laterResult = await runCliAgent(baseRunParams);
291+
expect(laterResult.didSendViaMessagingTool).toBeUndefined();
292+
expect(laterResult.messagingToolSentTargets).toBeUndefined();
293+
});
294+
265295
it("can close temporary CLI live sessions after a run", async () => {
266296
executePreparedCliRunMock.mockResolvedValue({ text: "real reply" });
267297

@@ -282,4 +312,27 @@ describe("runCliAgent cron before_agent_reply seam", () => {
282312
expect(executePreparedCliRunMock).toHaveBeenCalledTimes(1);
283313
expect(closeMcpLoopbackServerMock).toHaveBeenCalledTimes(1);
284314
});
315+
316+
it("preserves confirmed delivery when bundle MCP cleanup fails", async () => {
317+
executePreparedCliRunMock.mockResolvedValue({
318+
text: "",
319+
didSendViaMessagingTool: true,
320+
});
321+
closeMcpLoopbackServerMock.mockRejectedValue(new Error("loopback cleanup failed"));
322+
323+
await expect(
324+
runCliAgent({ ...baseRunParams, cleanupBundleMcpOnRunEnd: true }),
325+
).resolves.toMatchObject({
326+
didSendViaMessagingTool: true,
327+
});
328+
});
329+
330+
it("surfaces bundle MCP cleanup failures when nothing was delivered", async () => {
331+
executePreparedCliRunMock.mockResolvedValue({ text: "real reply" });
332+
closeMcpLoopbackServerMock.mockRejectedValue(new Error("loopback cleanup failed"));
333+
334+
await expect(runCliAgent({ ...baseRunParams, cleanupBundleMcpOnRunEnd: true })).rejects.toThrow(
335+
"loopback cleanup failed",
336+
);
337+
});
285338
});

‎src/agents/cli-runner.helpers.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,33 @@ describe("resolveCliRunQueueKey", () => {
562562
}),
563563
).toBe("claude-cli:run-4");
564564
});
565+
566+
it("keeps Claude live sessions serialized when serialize=false", () => {
567+
expect(
568+
resolveCliRunQueueKey({
569+
backendId: "claude-cli",
570+
liveSession: "claude-stdio",
571+
serialize: false,
572+
runId: "run-live",
573+
workspaceDir: "/tmp/project-a",
574+
ownerKey: "abcd1234",
575+
}),
576+
).toBe("claude-cli:owner:abcd1234");
577+
});
578+
579+
it("keeps resumed Claude live sessions on the owner lane", () => {
580+
expect(
581+
resolveCliRunQueueKey({
582+
backendId: "claude-cli",
583+
liveSession: "claude-stdio",
584+
serialize: true,
585+
runId: "run-live-resumed",
586+
workspaceDir: "/tmp/project-a",
587+
cliSessionId: "claude-session-123",
588+
ownerKey: "abcd1234",
589+
}),
590+
).toBe("claude-cli:owner:abcd1234");
591+
});
565592
});
566593

567594
describe("buildClaudeOwnerKey", () => {

0 commit comments

Comments
 (0)