From 0ea08076c3b58b074e9c9f8ab27d41ef86d0abf3 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 16 Jun 2026 02:27:12 +0800 Subject: [PATCH] fix(agents): preserve CLI message delivery evidence --- extensions/slack/src/channel-actions.ts | 12 + extensions/slack/src/message-tools.test.ts | 11 + .../telegram/src/channel-actions.test.ts | 9 + extensions/telegram/src/channel-actions.ts | 19 + extensions/telegram/src/channel.ts | 4 + src/agents/cli-output.test.ts | 16 + src/agents/cli-output.ts | 23 +- ...cli-runner.before-agent-reply-cron.test.ts | 59 +- src/agents/cli-runner.helpers.test.ts | 27 + src/agents/cli-runner.reliability.test.ts | 728 +++++++++++ src/agents/cli-runner.spawn.test.ts | 230 +++- src/agents/cli-runner.ts | 371 +++++- src/agents/cli-runner/bundle-mcp-gemini.ts | 43 + .../cli-runner/bundle-mcp.codex.test.ts | 5 +- .../cli-runner/bundle-mcp.gemini.test.ts | 49 +- src/agents/cli-runner/bundle-mcp.ts | 25 +- src/agents/cli-runner/claude-live-session.ts | 53 +- src/agents/cli-runner/delivery-evidence.ts | 79 ++ .../execute.supervisor-capture.test.ts | 760 +++++++++++ src/agents/cli-runner/execute.ts | 1129 ++++++++++++----- .../cli-runner/helpers.system-prompt.test.ts | 13 + src/agents/cli-runner/helpers.ts | 12 +- src/agents/cli-runner/prepare.test.ts | 147 ++- src/agents/cli-runner/prepare.ts | 28 +- src/agents/cli-runner/types.ts | 2 + src/agents/cli-session.test.ts | 25 + src/agents/cli-session.ts | 10 + .../command/attempt-execution.cli.test.ts | 48 + src/agents/command/attempt-execution.ts | 3 + ...ed-agent-message-tool-source-reply.test.ts | 298 +++++ ...mbedded-agent-message-tool-source-reply.ts | 366 +++++- src/agents/embedded-agent-messaging.ts | 49 +- ...embedded-agent-subscribe.handlers.tools.ts | 164 +-- src/agents/embedded-agent-subscribe.tools.ts | 148 ++- src/agents/openclaw-tools.sessions.test.ts | 39 + src/agents/system-prompt.test.ts | 16 + src/agents/system-prompt.ts | 7 +- src/agents/tool-loop-detection.test.ts | 12 + src/agents/tools/sessions-send-tool.ts | 3 + .../agent-runner.runreplyagent.e2e.test.ts | 48 + src/auto-reply/reply/agent-runner.ts | 3 + src/channels/plugins/types.core.ts | 2 + src/config/sessions/types.ts | 1 + src/gateway/mcp-http.handlers.ts | 28 + src/gateway/mcp-http.loopback-runtime.ts | 311 +++++ src/gateway/mcp-http.request.ts | 4 + src/gateway/mcp-http.test.ts | 365 ++++++ src/gateway/mcp-http.ts | 94 +- src/infra/outbound/internal-source-reply.ts | 120 ++ ...sage-action-runner.plugin-dispatch.test.ts | 128 ++ src/infra/outbound/message-action-runner.ts | 83 +- src/infra/outbound/message.test.ts | 13 + src/infra/outbound/message.ts | 2 + 53 files changed, 5561 insertions(+), 683 deletions(-) create mode 100644 src/agents/cli-runner/delivery-evidence.ts create mode 100644 src/agents/embedded-agent-message-tool-source-reply.test.ts create mode 100644 src/infra/outbound/internal-source-reply.ts diff --git a/extensions/slack/src/channel-actions.ts b/extensions/slack/src/channel-actions.ts index dfd3c61899f9..8ca8bd49053b 100644 --- a/extensions/slack/src/channel-actions.ts +++ b/extensions/slack/src/channel-actions.ts @@ -15,6 +15,16 @@ type SlackActionInvoke = ( let slackActionRuntimePromise: Promise | undefined; +const SLACK_TOOL_DELIVERY_ACTIONS = new Set([ + "deleteMessage", + "editMessage", + "pinMessage", + "react", + "sendMessage", + "unpinMessage", + "uploadFile", +]); + async function loadSlackActionRuntime() { slackActionRuntimePromise ??= import("./action-runtime.runtime.js"); return await slackActionRuntimePromise; @@ -42,6 +52,8 @@ export function createSlackActions( return { describeMessageTool: describeSlackMessageTool, extractToolSend: ({ args }) => extractSlackToolSend(args), + isToolDeliveryAction: ({ args }) => + typeof args.action === "string" && SLACK_TOOL_DELIVERY_ACTIONS.has(args.action), prepareSendPayload: ({ ctx, payload }) => (ctx.action === "send" ? payload : null), handleAction: async (ctx) => { return await handleSlackMessageAction({ diff --git a/extensions/slack/src/message-tools.test.ts b/extensions/slack/src/message-tools.test.ts index 9ddb330e7c67..3ba5ff1bc929 100644 --- a/extensions/slack/src/message-tools.test.ts +++ b/extensions/slack/src/message-tools.test.ts @@ -1,6 +1,7 @@ // Slack tests cover message tools plugin behavior. import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { describe, expect, it } from "vitest"; +import { createSlackActions } from "./channel-actions.js"; import { listSlackMessageActions } from "./message-actions.js"; import { describeSlackMessageTool } from "./message-tool-api.js"; @@ -24,6 +25,16 @@ function requireSchemaProperty( } describe("Slack message tools", () => { + it("classifies provider-native mutation actions", () => { + const actions = createSlackActions("slack"); + for (const action of ["sendMessage", "editMessage", "deleteMessage", "pinMessage"]) { + expect(actions.isToolDeliveryAction?.({ args: { action } })).toBe(true); + } + for (const action of ["readMessages", "listPins", "downloadFile"]) { + expect(actions.isToolDeliveryAction?.({ args: { action } })).toBe(false); + } + }); + it("describes configured Slack message actions without loading channel runtime", () => { const discovery = describeSlackMessageTool({ cfg: { diff --git a/extensions/telegram/src/channel-actions.test.ts b/extensions/telegram/src/channel-actions.test.ts index be3a7977ec41..878297dc0dcc 100644 --- a/extensions/telegram/src/channel-actions.test.ts +++ b/extensions/telegram/src/channel-actions.test.ts @@ -27,6 +27,15 @@ describe("telegramMessageActions", () => { } }); + it("classifies provider-native mutation actions", () => { + for (const action of ["sendMessage", "editMessage", "deleteMessage", "react", "topic-edit"]) { + expect(telegramMessageActions.isToolDeliveryAction?.({ args: { action } })).toBe(true); + } + for (const action of ["searchSticker", "stickerCacheStats"]) { + expect(telegramMessageActions.isToolDeliveryAction?.({ args: { action } })).toBe(false); + } + }); + it("allows interactive-only sends", async () => { await telegramMessageActions.handleAction!({ action: "send", diff --git a/extensions/telegram/src/channel-actions.ts b/extensions/telegram/src/channel-actions.ts index 6053fbdf685c..457f0f70cd4d 100644 --- a/extensions/telegram/src/channel-actions.ts +++ b/extensions/telegram/src/channel-actions.ts @@ -50,6 +50,23 @@ const TELEGRAM_MESSAGE_ACTION_MAP = { "topic-edit": "editForumTopic", } as const satisfies Partial>; +const TELEGRAM_TOOL_DELIVERY_ACTIONS = new Set([ + "createForumTopic", + "delete", + "deleteMessage", + "edit", + "editForumTopic", + "editMessage", + "poll", + "react", + "send", + "sendMessage", + "sendSticker", + "sticker", + "topic-create", + "topic-edit", +]); + function resolveTelegramMessageActionName(action: ChannelMessageActionName) { return TELEGRAM_MESSAGE_ACTION_MAP[action as keyof typeof TELEGRAM_MESSAGE_ACTION_MAP]; } @@ -181,6 +198,8 @@ export const telegramMessageActions: ChannelMessageActionAdapter = { extractToolSend: ({ args }) => { return extractToolSend(args, "sendMessage"); }, + isToolDeliveryAction: ({ args }) => + typeof args.action === "string" && TELEGRAM_TOOL_DELIVERY_ACTIONS.has(args.action), handleAction: async ({ action, params, diff --git a/extensions/telegram/src/channel.ts b/extensions/telegram/src/channel.ts index e96c764e1266..135f6cdaaf24 100644 --- a/extensions/telegram/src/channel.ts +++ b/extensions/telegram/src/channel.ts @@ -289,6 +289,10 @@ const telegramMessageActions: ChannelMessageActionAdapter = { getOptionalTelegramRuntime()?.channel?.telegram?.messageActions?.extractToolSend?.(ctx) ?? telegramMessageActionsImpl.extractToolSend?.(ctx) ?? null, + isToolDeliveryAction: (ctx) => + getOptionalTelegramRuntime()?.channel?.telegram?.messageActions?.isToolDeliveryAction?.(ctx) ?? + telegramMessageActionsImpl.isToolDeliveryAction?.(ctx) ?? + false, handleAction: async (ctx) => { const runtimeHandleAction = getOptionalTelegramRuntime()?.channel?.telegram?.messageActions?.handleAction; diff --git a/src/agents/cli-output.test.ts b/src/agents/cli-output.test.ts index afc502893eb7..2af960f51e78 100644 --- a/src/agents/cli-output.test.ts +++ b/src/agents/cli-output.test.ts @@ -5,10 +5,26 @@ import { extractCliErrorMessage, parseCliJson, parseCliJsonl, + supportsCliJsonlToolEvents, type CliToolUseStartDelta, } from "./cli-output.js"; import { createClaudeApiErrorFixture } from "./test-helpers/claude-api-error-fixture.js"; +describe("supportsCliJsonlToolEvents", () => { + it.each([ + ["Claude provider", { command: "claude", output: "jsonl" as const }, "claude-cli", true], + [ + "explicit Claude dialect", + { command: "custom", output: "jsonl" as const, jsonlDialect: "claude-stream-json" as const }, + "custom-cli", + true, + ], + ["generic JSONL", { command: "custom", output: "jsonl" as const }, "custom-cli", false], + ])("%s: %s", (_name, backend, providerId, expected) => { + expect(supportsCliJsonlToolEvents({ backend, providerId })).toBe(expected); + }); +}); + describe("parseCliJson", () => { it("recovers mixed-output Claude session metadata from embedded JSON objects", () => { const result = parseCliJson( diff --git a/src/agents/cli-output.ts b/src/agents/cli-output.ts index cfaae0954bbd..1d8175483eff 100644 --- a/src/agents/cli-output.ts +++ b/src/agents/cli-output.ts @@ -8,6 +8,10 @@ import { normalizeStringEntries } from "@openclaw/normalization-core/string-norm import type { CliBackendConfig } from "../config/types.js"; import { extractBalancedJsonFragments } from "../shared/balanced-json.js"; import { isRecord } from "../utils.js"; +import type { + MessagingToolSend, + MessagingToolSourceReplyPayload, +} from "./embedded-agent-messaging.types.js"; type CliUsage = { input?: number; @@ -24,6 +28,12 @@ export type CliOutput = { sessionId?: string; usage?: CliUsage; finalPromptText?: string; + didSendViaMessagingTool?: boolean; + didDeliverSourceReplyViaMessageTool?: boolean; + messagingToolSentTexts?: string[]; + messagingToolSentMediaUrls?: string[]; + messagingToolSentTargets?: MessagingToolSend[]; + messagingToolSourceReplyPayloads?: MessagingToolSourceReplyPayload[]; }; /** Incremental assistant text emitted while parsing a streaming CLI response. */ @@ -53,7 +63,8 @@ function isClaudeCliProvider(providerId: string): boolean { return normalizeLowercaseStringOrEmpty(providerId) === "claude-cli"; } -function usesClaudeStreamJsonDialect(params: { +/** Returns whether JSONL output carries correlated Claude-style tool events. */ +export function supportsCliJsonlToolEvents(params: { backend: CliBackendConfig; providerId: string; }): boolean { @@ -67,7 +78,7 @@ function isClaudeStreamJsonResult(params: { providerId: string; parsed: Record; }): boolean { - return usesClaudeStreamJsonDialect(params) && params.parsed.type === "result"; + return supportsCliJsonlToolEvents(params) && params.parsed.type === "result"; } function extractJsonObjectCandidates(raw: string): string[] { @@ -368,7 +379,7 @@ function parseClaudeCliJsonlResult(params: { sessionId?: string; usage?: CliUsage; }): CliOutput | null { - if (!usesClaudeStreamJsonDialect(params)) { + if (!supportsCliJsonlToolEvents(params)) { return null; } if ( @@ -395,7 +406,7 @@ function parseClaudeCliStreamingDelta(params: { sessionId?: string; usage?: CliUsage; }): CliStreamingDelta | null { - if (!usesClaudeStreamJsonDialect(params)) { + if (!supportsCliJsonlToolEvents(params)) { return null; } if (params.parsed.type !== "stream_event" || !isRecord(params.parsed.event)) { @@ -510,7 +521,7 @@ function dispatchClaudeCliStreamingToolEvent(params: { onToolUseStart?: (delta: CliToolUseStartDelta) => void; onToolResult?: (delta: CliToolResultDelta) => void; }): void { - if (!usesClaudeStreamJsonDialect(params)) { + if (!supportsCliJsonlToolEvents(params)) { return; } const tracker = params.tracker; @@ -644,7 +655,7 @@ export function createCliJsonlStreamingParser(params: { // Classification is keyed on consumer presence so reclassified pre-tool text // always has a destination; a separate enable flag let it be dropped (#92092). const classifyClaudeCommentary = - Boolean(params.onCommentaryText) && usesClaudeStreamJsonDialect(params); + Boolean(params.onCommentaryText) && supportsCliJsonlToolEvents(params); const flushPendingClaudeAssistantText = () => { if (!pendingClaudeText) { diff --git a/src/agents/cli-runner.before-agent-reply-cron.test.ts b/src/agents/cli-runner.before-agent-reply-cron.test.ts index 5b85ebca283f..d62da76703ec 100644 --- a/src/agents/cli-runner.before-agent-reply-cron.test.ts +++ b/src/agents/cli-runner.before-agent-reply-cron.test.ts @@ -1,6 +1,7 @@ /** Tests cron before_agent_reply gating at the CLI runner entrypoint. */ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; +import type { CliOutput } from "./cli-output.js"; import { cliBackendLog } from "./cli-runner/log.js"; // vi.mock factories are hoisted above imports, so any references inside them @@ -28,9 +29,9 @@ const { runBeforeAgentReplyMock: vi.fn<(event: unknown, ctx: unknown) => Promise>( async () => undefined, ), - executePreparedCliRunMock: vi.fn(async (_context: unknown, _cliSessionIdToUse?: string) => ({ - text: "", - })), + executePreparedCliRunMock: vi.fn< + (_context: unknown, _cliSessionIdToUse?: string) => Promise + >(async () => ({ text: "" })), prepareCliRunContextMock: vi.fn(), closeClaudeLiveSessionForContextMock: vi.fn(), closeMcpLoopbackServerMock: vi.fn(), @@ -262,6 +263,35 @@ describe("runCliAgent cron before_agent_reply seam", () => { expect(executePreparedCliRunMock).toHaveBeenCalledTimes(1); }); + it("reports confirmed CLI messaging delivery evidence without leaking it to later invocations", async () => { + executePreparedCliRunMock.mockResolvedValueOnce({ + text: "sent", + didSendViaMessagingTool: true, + messagingToolSentTargets: [ + { + tool: "message", + provider: "telegram", + to: "chat123", + }, + ], + }); + executePreparedCliRunMock.mockResolvedValueOnce({ text: "later" }); + + const firstResult = await runCliAgent(baseRunParams); + expect(firstResult.didSendViaMessagingTool).toBe(true); + expect(firstResult.messagingToolSentTargets).toEqual([ + expect.objectContaining({ + tool: "message", + provider: "telegram", + to: "chat123", + }), + ]); + + const laterResult = await runCliAgent(baseRunParams); + expect(laterResult.didSendViaMessagingTool).toBeUndefined(); + expect(laterResult.messagingToolSentTargets).toBeUndefined(); + }); + it("can close temporary CLI live sessions after a run", async () => { executePreparedCliRunMock.mockResolvedValue({ text: "real reply" }); @@ -282,4 +312,27 @@ describe("runCliAgent cron before_agent_reply seam", () => { expect(executePreparedCliRunMock).toHaveBeenCalledTimes(1); expect(closeMcpLoopbackServerMock).toHaveBeenCalledTimes(1); }); + + it("preserves confirmed delivery when bundle MCP cleanup fails", async () => { + executePreparedCliRunMock.mockResolvedValue({ + text: "", + didSendViaMessagingTool: true, + }); + closeMcpLoopbackServerMock.mockRejectedValue(new Error("loopback cleanup failed")); + + await expect( + runCliAgent({ ...baseRunParams, cleanupBundleMcpOnRunEnd: true }), + ).resolves.toMatchObject({ + didSendViaMessagingTool: true, + }); + }); + + it("surfaces bundle MCP cleanup failures when nothing was delivered", async () => { + executePreparedCliRunMock.mockResolvedValue({ text: "real reply" }); + closeMcpLoopbackServerMock.mockRejectedValue(new Error("loopback cleanup failed")); + + await expect(runCliAgent({ ...baseRunParams, cleanupBundleMcpOnRunEnd: true })).rejects.toThrow( + "loopback cleanup failed", + ); + }); }); diff --git a/src/agents/cli-runner.helpers.test.ts b/src/agents/cli-runner.helpers.test.ts index e416a23e3924..ce6eefa59a97 100644 --- a/src/agents/cli-runner.helpers.test.ts +++ b/src/agents/cli-runner.helpers.test.ts @@ -562,6 +562,33 @@ describe("resolveCliRunQueueKey", () => { }), ).toBe("claude-cli:run-4"); }); + + it("keeps Claude live sessions serialized when serialize=false", () => { + expect( + resolveCliRunQueueKey({ + backendId: "claude-cli", + liveSession: "claude-stdio", + serialize: false, + runId: "run-live", + workspaceDir: "/tmp/project-a", + ownerKey: "abcd1234", + }), + ).toBe("claude-cli:owner:abcd1234"); + }); + + it("keeps resumed Claude live sessions on the owner lane", () => { + expect( + resolveCliRunQueueKey({ + backendId: "claude-cli", + liveSession: "claude-stdio", + serialize: true, + runId: "run-live-resumed", + workspaceDir: "/tmp/project-a", + cliSessionId: "claude-session-123", + ownerKey: "abcd1234", + }), + ).toBe("claude-cli:owner:abcd1234"); + }); }); describe("buildClaudeOwnerKey", () => { diff --git a/src/agents/cli-runner.reliability.test.ts b/src/agents/cli-runner.reliability.test.ts index 806cd93a8ac2..76306349ba69 100644 --- a/src/agents/cli-runner.reliability.test.ts +++ b/src/agents/cli-runner.reliability.test.ts @@ -12,7 +12,16 @@ import { import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; import { CURRENT_SESSION_VERSION } from "../config/sessions/version.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { + markMcpLoopbackRequestClassified, + markMcpLoopbackRequestStarted, + markMcpLoopbackToolCallFinished, + markMcpLoopbackToolCallStarted, + recordMcpLoopbackToolCallResult, + updateMcpLoopbackToolCallCapture, +} from "../gateway/mcp-http.loopback-runtime.js"; import { getGlobalHookRunner } from "../plugins/hook-runner-global.js"; +import type { getProcessSupervisor } from "../process/supervisor/index.js"; import { createUserTurnTranscriptRecorder, type UserTurnTranscriptRecorder, @@ -572,6 +581,725 @@ describe("runCliAgent reliability", () => { expect(supervisorSpawnMock).toHaveBeenCalledTimes(2); }); + it("does not retry or fail over after a confirmed message send", async () => { + supervisorSpawnMock.mockClear(); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureKey = input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? ""; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey, + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + mediaUrl: "https://example.com/done.png", + }, + }); + if (!captureHandle) { + throw new Error("Expected message delivery capture"); + } + setTimeout(() => { + recordMcpLoopbackToolCallResult({ + captureHandle, + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + mediaUrl: "https://example.com/done.png", + }, + result: { status: "sent" }, + isError: false, + }); + markMcpLoopbackToolCallFinished(captureHandle); + }, 10); + return createManagedRun({ + reason: "no-output-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 200, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: true, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:delivered-timeout", + runId: "run-delivered-timeout", + cliSessionId: "stale-cli-session", + provider: "claude-cli", + model: "opus", + openClawHistoryPrompt: CLI_RESEED_PROMPT, + }); + context.mcpDeliveryCapture = true; + + const result = await runPreparedCliAgent(context); + + expect(result.payloads).toBeUndefined(); + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.messagingToolSentTexts).toEqual(["done"]); + expect(result.messagingToolSentMediaUrls).toEqual(["https://example.com/done.png"]); + expect(result.messagingToolSentTargets).toEqual([ + expect.objectContaining({ tool: "message", provider: "telegram", to: "chat123" }), + ]); + expect(result.meta.executionTrace?.attempts?.[0]?.result).toBe("error"); + expect(result.meta.agentMeta?.clearCliSessionBinding).toBe(true); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + }); + + it("preserves first-turn delivery through cleanup without binding the OpenClaw session id", async () => { + supervisorSpawnMock.mockClear(); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + message: "sent before failure", + }, + }); + if (!captureHandle) { + throw new Error("Expected message delivery capture"); + } + recordMcpLoopbackToolCallResult({ + captureHandle, + toolName: "message", + args: { + action: "send", + message: "sent before failure", + }, + result: { + details: { + deliveryStatus: "sent", + sourceReplySink: "internal-ui", + sourceReply: { text: "sent before failure" }, + }, + }, + isError: false, + }); + markMcpLoopbackToolCallFinished(captureHandle); + return createManagedRun({ + reason: "no-output-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 200, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: true, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:first-turn-delivered", + runId: "run-first-turn-delivered", + provider: "claude-cli", + model: "opus", + }); + context.mcpDeliveryCapture = true; + context.params.sourceReplyDeliveryMode = "message_tool_only"; + context.preparedBackend.cleanup = async () => { + throw new Error("cleanup failed"); + }; + + const result = await runPreparedCliAgent(context); + + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.didDeliverSourceReplyViaMessageTool).toBe(true); + expect(result.messagingToolSourceReplyPayloads).toEqual([{ text: "sent before failure" }]); + expect(result.payloads).toEqual([{ text: "sent before failure" }]); + expect(getReplyPayloadMetadata(result.payloads?.[0] as object)).toMatchObject({ + deliverDespiteSourceReplySuppression: true, + sourceReplyTranscriptMirror: { + sessionKey: "agent:main:first-turn-delivered", + text: "sent before failure", + idempotencyKey: "run-first-turn-delivered:internal-source-reply:0", + }, + }); + expect(result.meta.agentMeta?.sessionId).toBe(""); + expect(result.meta.agentMeta?.clearCliSessionBinding).toBeUndefined(); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + }); + + it("returns only the source-reply mirror after a successful CLI turn", async () => { + supervisorSpawnMock.mockClear(); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + message: "sent through source reply", + }, + }); + if (!captureHandle) { + throw new Error("Expected message delivery capture"); + } + recordMcpLoopbackToolCallResult({ + captureHandle, + toolName: "message", + args: { + action: "send", + message: "sent through source reply", + }, + result: { + details: { + deliveryStatus: "sent", + sourceReplySink: "internal-ui", + sourceReply: { text: "sent through source reply" }, + }, + }, + isError: false, + }); + markMcpLoopbackToolCallFinished(captureHandle); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "ordinary final should stay private", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:successful-source-reply", + runId: "run-successful-source-reply", + provider: "claude-cli", + model: "opus", + }); + context.mcpDeliveryCapture = true; + context.params.sourceReplyDeliveryMode = "message_tool_only"; + + const result = await runPreparedCliAgent(context); + + expect(result.payloads).toEqual([{ text: "sent through source reply" }]); + expect(getReplyPayloadMetadata(result.payloads?.[0] as object)).toMatchObject({ + deliverDespiteSourceReplySuppression: true, + sourceReplyTranscriptMirror: { + sessionKey: "agent:main:successful-source-reply", + text: "sent through source reply", + idempotencyKey: "run-successful-source-reply:internal-source-reply:0", + }, + }); + expect(result.meta.finalAssistantVisibleText).toBe("sent through source reply"); + }); + + it("hooks the visible source reply without pre-persisting its dispatch mirror", async () => { + const { dir, sessionFile, storePath } = createSessionFile(); + const hookRunner = { + hasHooks: vi.fn((hookName: string) => ["llm_output", "agent_end"].includes(hookName)), + runLlmInput: vi.fn(async () => undefined), + runLlmOutput: vi.fn(async () => undefined), + runAgentEnd: vi.fn(async () => undefined), + }; + setHookRunnerForTest(hookRunner); + supervisorSpawnMock.mockClear(); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + message: "visible source reply", + }, + }); + if (!captureHandle) { + throw new Error("Expected message delivery capture"); + } + recordMcpLoopbackToolCallResult({ + captureHandle, + toolName: "message", + args: { + action: "send", + message: "visible source reply", + }, + result: { + details: { + deliveryStatus: "sent", + sourceReplySink: "internal-ui", + sourceReply: { text: "visible source reply" }, + }, + }, + isError: false, + }); + markMcpLoopbackToolCallFinished(captureHandle); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "private terminal confirmation", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:main", + runId: "run-visible-source-reply", + provider: "claude-cli", + model: "opus", + }); + context.mcpDeliveryCapture = true; + context.params.sourceReplyDeliveryMode = "message_tool_only"; + context.params.sessionFile = sessionFile; + context.params.storePath = storePath; + context.params.persistAssistantTranscript = true; + + try { + await runPreparedCliAgent(context); + + const transcriptMessages = readTranscriptMessages(sessionFile); + expect(transcriptMessages).toHaveLength(0); + const llmOutputEvent = requireRecord( + callArg(hookRunner.runLlmOutput, 0, 0, "llm_output event"), + "llm_output event", + ); + expect(llmOutputEvent.assistantTexts).toEqual(["visible source reply"]); + const agentEndEvent = requireRecord( + callArg(hookRunner.runAgentEnd, 0, 0, "agent_end event"), + "agent_end event", + ); + const messages = requireArray(agentEndEvent.messages, "agent_end messages"); + const lastMessage = requireRecord(messages.at(-1), "agent_end assistant message"); + expect(lastMessage.role).toBe("assistant"); + expect(lastMessage.content).toEqual([{ type: "text", text: "visible source reply" }]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it("accepts empty terminal output after a confirmed message delivery", async () => { + supervisorSpawnMock.mockClear(); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "sent without a terminal reply", + }, + }); + if (!captureHandle) { + throw new Error("Expected message delivery capture"); + } + recordMcpLoopbackToolCallResult({ + captureHandle, + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "sent without a terminal reply", + }, + result: { status: "sent" }, + isError: false, + }); + markMcpLoopbackToolCallFinished(captureHandle); + input.onStdout?.( + `${JSON.stringify({ type: "result", session_id: "claude-session", result: "" })}\n`, + ); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:successful-empty-delivery", + runId: "run-successful-empty-delivery", + provider: "claude-cli", + model: "opus", + }); + context.backendResolved.config.output = "jsonl"; + context.mcpDeliveryCapture = true; + + const result = await runPreparedCliAgent(context); + + expect(result.payloads).toBeUndefined(); + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.meta.executionTrace?.attempts?.[0]?.result).toBe("success"); + }); + + it("keeps unresolved internal source replies retryable", async () => { + vi.useFakeTimers(); + supervisorSpawnMock.mockClear(); + let captureStarted: (() => void) | undefined; + const captureStartedPromise = new Promise((resolve) => { + captureStarted = resolve; + }); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + message: "pending internal source reply", + }, + }); + if (!captureHandle) { + throw new Error("Expected internal source reply capture"); + } + updateMcpLoopbackToolCallCapture(captureHandle, { + toolName: "message", + args: { + action: "send", + message: "pending internal source reply", + }, + }); + captureStarted?.(); + return createManagedRun({ + reason: "no-output-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 200, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: true, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:unresolved-internal-source-reply", + runId: "run-unresolved-internal-source-reply", + provider: "claude-cli", + model: "opus", + }); + context.mcpDeliveryCapture = true; + context.params.config = {}; + context.params.messageChannel = "webchat"; + context.params.sourceReplyDeliveryMode = "message_tool_only"; + + const resultPromise = runPreparedCliAgent(context); + const resultAssertion = expect(resultPromise).rejects.toThrow("CLI produced no output"); + await captureStartedPromise; + await vi.runAllTimersAsync(); + await resultAssertion; + + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + }); + + it("fails closed when an unresolved implicit send resolves to an external session route", async () => { + vi.useFakeTimers(); + supervisorSpawnMock.mockClear(); + let captureStarted: (() => void) | undefined; + const captureStartedPromise = new Promise((resolve) => { + captureStarted = resolve; + }); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + message: "pending external session reply", + }, + }); + if (!captureHandle) { + throw new Error("Expected external session reply capture"); + } + updateMcpLoopbackToolCallCapture(captureHandle, { + toolName: "message", + args: { + action: "send", + message: "pending external session reply", + }, + }); + captureStarted?.(); + return createManagedRun({ + reason: "no-output-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 200, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: true, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:telegram:direct:123456789", + runId: "run-unresolved-external-session-reply", + provider: "claude-cli", + model: "opus", + }); + context.mcpDeliveryCapture = true; + context.params.config = {}; + context.params.messageChannel = "webchat"; + context.params.sourceReplyDeliveryMode = "message_tool_only"; + + const resultPromise = runPreparedCliAgent(context); + await captureStartedPromise; + await vi.runAllTimersAsync(); + const result = await resultPromise; + + expect(result.didSendViaMessagingTool).toBe(true); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + }); + + it("surfaces prepared backend cleanup failures when nothing was delivered", async () => { + supervisorSpawnMock.mockResolvedValueOnce( + createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "ok", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }), + ); + const context = buildPreparedContext({ + sessionKey: "agent:main:cleanup-failure", + runId: "run-cleanup-failure", + }); + context.preparedBackend.cleanup = async () => { + throw new Error("cleanup failed"); + }; + + await expect(runPreparedCliAgent(context)).rejects.toThrow("cleanup failed"); + }); + + it("bounds unresolved message sends and does not retry them", async () => { + vi.useFakeTimers(); + supervisorSpawnMock.mockClear(); + let captureStarted: (() => void) | undefined; + const captureStartedPromise = new Promise((resolve) => { + captureStarted = resolve; + }); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "react", + channel: "telegram", + target: "chat123", + }, + }); + if (!captureHandle) { + throw new Error("Expected message delivery capture"); + } + updateMcpLoopbackToolCallCapture(captureHandle, { + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "possibly sent", + }, + }); + captureStarted?.(); + return createManagedRun({ + reason: "no-output-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 200, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: true, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:unresolved-send", + runId: "run-unresolved-send", + cliSessionId: "stale-cli-session", + provider: "claude-cli", + model: "opus", + openClawHistoryPrompt: CLI_RESEED_PROMPT, + }); + context.mcpDeliveryCapture = true; + + const resultPromise = runPreparedCliAgent(context); + await captureStartedPromise; + await vi.runAllTimersAsync(); + const result = await resultPromise; + + expect(result.payloads).toBeUndefined(); + expect(result.didSendViaMessagingTool).toBe(true); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + }); + + it("bounds admitted requests that have not finished uploading", async () => { + vi.useFakeTimers(); + supervisorSpawnMock.mockClear(); + let captureStarted: (() => void) | undefined; + const captureStartedPromise = new Promise((resolve) => { + captureStarted = resolve; + }); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackRequestStarted( + input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + ); + if (!captureHandle) { + throw new Error("Expected request delivery capture"); + } + captureStarted?.(); + return createManagedRun({ + reason: "no-output-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 200, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: true, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:unresolved-request", + runId: "run-unresolved-request", + cliSessionId: "stale-cli-session", + provider: "claude-cli", + model: "opus", + openClawHistoryPrompt: CLI_RESEED_PROMPT, + }); + context.mcpDeliveryCapture = true; + + const resultPromise = runPreparedCliAgent(context); + await captureStartedPromise; + await vi.runAllTimersAsync(); + const result = await resultPromise; + + expect(result.payloads).toBeUndefined(); + expect(result.didSendViaMessagingTool).toBe(true); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + }); + + it("does not treat classified non-message requests as delivery", async () => { + vi.useFakeTimers(); + supervisorSpawnMock.mockClear(); + let captureStarted: (() => void) | undefined; + const captureStartedPromise = new Promise((resolve) => { + captureStarted = resolve; + }); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const requestCaptureHandle = markMcpLoopbackRequestStarted( + input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + ); + if (!requestCaptureHandle) { + throw new Error("Expected request delivery capture"); + } + markMcpLoopbackToolCallStarted({ + requestCaptureHandle, + toolName: "exec", + args: { command: "sleep 30" }, + }); + markMcpLoopbackRequestClassified(requestCaptureHandle); + captureStarted?.(); + return createManagedRun({ + reason: "no-output-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 200, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: true, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:unresolved-non-message-request", + runId: "run-unresolved-non-message-request", + cliSessionId: "stale-cli-session", + provider: "claude-cli", + model: "opus", + openClawHistoryPrompt: CLI_RESEED_PROMPT, + }); + context.mcpDeliveryCapture = true; + + const resultPromise = runPreparedCliAgent(context); + const resultAssertion = expect(resultPromise).rejects.toThrow("produced no output"); + await captureStartedPromise; + await vi.runAllTimersAsync(); + await resultAssertion; + + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + }); + + it("fails normally after an unresolved prepared dry-run send", async () => { + vi.useFakeTimers(); + supervisorSpawnMock.mockClear(); + let captureStarted: (() => void) | undefined; + const captureStartedPromise = new Promise((resolve) => { + captureStarted = resolve; + }); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "preview", + }, + }); + updateMcpLoopbackToolCallCapture(captureHandle, { + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "preview", + dryRun: true, + }, + }); + captureStarted?.(); + return createManagedRun({ + reason: "no-output-timeout", + exitCode: null, + exitSignal: "SIGKILL", + durationMs: 200, + stdout: "", + stderr: "", + timedOut: true, + noOutputTimedOut: true, + }); + }); + const context = buildPreparedContext({ + sessionKey: "agent:main:unresolved-dry-run", + runId: "run-unresolved-dry-run", + cliSessionId: "stale-cli-session", + provider: "claude-cli", + model: "opus", + openClawHistoryPrompt: CLI_RESEED_PROMPT, + }); + context.mcpDeliveryCapture = true; + + const resultPromise = runPreparedCliAgent(context); + const resultAssertion = expect(resultPromise).rejects.toThrow("produced no output"); + await captureStartedPromise; + await vi.runAllTimersAsync(); + await resultAssertion; + + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + }); + it("does not retry an unclassified CLI failure with diagnostic output", async () => { supervisorSpawnMock.mockClear(); const clearBeforeRetry = vi.fn(async () => true); diff --git a/src/agents/cli-runner.spawn.test.ts b/src/agents/cli-runner.spawn.test.ts index b6aeca16e103..df2e4bea10ee 100644 --- a/src/agents/cli-runner.spawn.test.ts +++ b/src/agents/cli-runner.spawn.test.ts @@ -8,6 +8,11 @@ import { createReplyOperation, replyRunRegistry, } from "../auto-reply/reply/reply-run-registry.js"; +import { + markMcpLoopbackToolCallFinished, + markMcpLoopbackToolCallStarted, + recordMcpLoopbackToolCallResult, +} from "../gateway/mcp-http.loopback-runtime.js"; import { onAgentEvent, resetAgentEventsForTest } from "../infra/agent-events.js"; import { onInternalDiagnosticEvent, @@ -18,6 +23,7 @@ import { resetDiagnosticRunActivityForTest, } from "../logging/diagnostic-run-activity.js"; import type { getProcessSupervisor } from "../process/supervisor/index.js"; +import type { RunExit } from "../process/supervisor/types.js"; import { makeBootstrapWarn as realMakeBootstrapWarn, resolveBootstrapContextForRun as realResolveBootstrapContextForRun, @@ -34,12 +40,17 @@ import { resetClaudeLiveSessionsForTest, runClaudeLiveSessionTurn, } from "./cli-runner/claude-live-session.js"; +import { + attachCliMessagingDeliveryEvidence, + getCliMessagingDeliveryEvidence, +} from "./cli-runner/delivery-evidence.js"; import { buildCliEnvAuthLog, buildCliExecLogLine, executePreparedCliRun, + setCliRunnerExecuteTestDeps, } from "./cli-runner/execute.js"; -import { buildSystemPrompt } from "./cli-runner/helpers.js"; +import { buildSystemPrompt, writeCliSystemPromptFile } from "./cli-runner/helpers.js"; import { cliBackendLog, formatCliBackendOutputDigest } from "./cli-runner/log.js"; import { setCliRunnerPrepareTestDeps } from "./cli-runner/prepare.js"; import type { PreparedCliRunContext } from "./cli-runner/types.js"; @@ -59,6 +70,7 @@ beforeEach(() => { resetClaudeLiveSessionsForTest(); replyRunTesting.resetReplyRunRegistry(); restoreCliRunnerPrepareTestDeps(); + setCliRunnerExecuteTestDeps({ writeCliSystemPromptFile }); supervisorSpawnMock.mockClear(); }); @@ -83,6 +95,7 @@ function buildPreparedCliRunContext(params: { resolveExecutionArgs?: PreparedCliRunContext["backendResolved"]["resolveExecutionArgs"]; config?: PreparedCliRunContext["params"]["config"]; mcpConfigHash?: string; + mcpDeliveryCapture?: boolean; skillsSnapshot?: PreparedCliRunContext["params"]["skillsSnapshot"]; thinkLevel?: PreparedCliRunContext["params"]["thinkLevel"]; executionMode?: PreparedCliRunContext["params"]["executionMode"]; @@ -161,6 +174,7 @@ function buildPreparedCliRunContext(params: { systemPromptReport: {} as PreparedCliRunContext["systemPromptReport"], bootstrapPromptWarningLines: [], authEpochVersion: 2, + ...(params.mcpDeliveryCapture ? { mcpDeliveryCapture: true } : {}), }; } @@ -700,6 +714,20 @@ describe("runCliAgent spawn path", () => { expect(params.currentInboundEventKind).toBe("room_event"); }); + it("forwards explicit message target policy through the compat wrapper", () => { + const params = buildRunClaudeCliAgentParams({ + sessionId: "openclaw-session", + sessionFile: "/tmp/session.jsonl", + workspaceDir: "/tmp", + prompt: "hi", + timeoutMs: 1_000, + runId: "run-claude-target-policy-wrapper", + requireExplicitMessageTarget: true, + }); + + expect(params.requireExplicitMessageTarget).toBe(true); + }); + it("forwards static extra system prompt through the compat wrapper", () => { const params = buildRunClaudeCliAgentParams({ sessionId: "openclaw-session", @@ -1178,6 +1206,148 @@ describe("runCliAgent spawn path", () => { await vi.waitFor(() => expect(preparedBackendCleanup).toHaveBeenCalledOnce()); }); + it("keeps captured live prepared backend cleanup with the whole-run owner", async () => { + let stdoutListener: ((chunk: string) => void) | undefined; + let resolveExit: ((exit: RunExit) => void) | undefined; + const exited = new Promise((resolve) => { + resolveExit = resolve; + }); + supervisorSpawnMock.mockImplementation(async (...args: unknown[]) => { + const input = (args[0] ?? {}) as { onStdout?: (chunk: string) => void }; + stdoutListener = input.onStdout; + return { + runId: "captured-live-cleanup-run", + pid: 2347, + startedAtMs: Date.now(), + stdin: { + write: vi.fn((dataValue: string, cb?: (err?: Error | null) => void) => { + stdoutListener?.( + [ + JSON.stringify({ + type: "system", + subtype: "init", + session_id: "captured-live-cleanup", + }), + JSON.stringify({ + type: "result", + session_id: "captured-live-cleanup", + result: "ok", + }), + ].join("\n") + "\n", + ); + cb?.(); + }), + end: vi.fn(), + }, + wait: vi.fn(() => exited), + cancel: vi.fn(() => + resolveExit?.({ + reason: "manual-cancel", + exitCode: null, + exitSignal: null, + durationMs: 1, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }), + ), + }; + }); + const preparedBackendCleanup = vi.fn(async () => {}); + const context = buildPreparedCliRunContext({ + provider: "claude-cli", + model: "sonnet", + runId: "run-captured-live-cleanup", + prompt: "first", + backend: { + args: ["-p", "--strict-mcp-config", "--mcp-config", "/tmp/mcp-captured.json"], + liveSession: "claude-stdio", + }, + mcpConfigHash: "captured-cleanup-mcp-config", + mcpDeliveryCapture: true, + }); + context.preparedBackend.cleanup = preparedBackendCleanup; + + const result = await executePreparedCliRun(context); + + expect(result.text).toBe("ok"); + expect(context.preparedBackend.cleanup).toBe(preparedBackendCleanup); + expect(preparedBackendCleanup).not.toHaveBeenCalled(); + + await context.preparedBackend.cleanup?.(); + expect(preparedBackendCleanup).toHaveBeenCalledOnce(); + }); + + it("preserves completed output when system prompt cleanup fails after delivery", async () => { + const cleanupError = new Error("system prompt cleanup failed"); + const logWarnSpy = vi.spyOn(cliBackendLog, "warn").mockImplementation(() => undefined); + setCliRunnerExecuteTestDeps({ + writeCliSystemPromptFile: async () => ({ + filePath: "/tmp/system-prompt.md", + cleanup: async () => { + throw cleanupError; + }, + }), + }); + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as Parameters["spawn"]>[0]; + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { action: "send", target: "chat123", message: "done" }, + }); + if (!captureHandle) { + throw new Error("Expected message delivery capture"); + } + recordMcpLoopbackToolCallResult({ + captureHandle, + toolName: "message", + args: { action: "send", target: "chat123", message: "done" }, + result: { status: "sent" }, + isError: false, + }); + markMcpLoopbackToolCallFinished(captureHandle); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + const context = buildPreparedCliRunContext({ + provider: "codex-cli", + model: "gpt-5.4", + runId: "run-cleanup-delivery-evidence", + mcpDeliveryCapture: true, + }); + + const result = await executePreparedCliRun(context); + setCliRunnerExecuteTestDeps({ writeCliSystemPromptFile }); + + expect(result.text).toBe("done"); + expect(result.didSendViaMessagingTool).toBe(true); + expect(logWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("outer resource cleanup failed after confirmed message delivery"), + ); + }); + + it("wraps primitive and frozen failures to preserve delivery evidence", () => { + const evidence = { didSendViaMessagingTool: true }; + const primitive = attachCliMessagingDeliveryEvidence("failed", evidence); + const frozen = attachCliMessagingDeliveryEvidence(Object.freeze(new Error("frozen")), evidence); + + expect(primitive).toBeInstanceOf(Error); + expect(frozen).toBeInstanceOf(Error); + expect(getCliMessagingDeliveryEvidence(primitive)?.didSendViaMessagingTool).toBe(true); + expect(getCliMessagingDeliveryEvidence(frozen)?.didSendViaMessagingTool).toBe(true); + }); + it("accepts Claude live stream-json lines larger than 256 KiB", async () => { const largeText = "x".repeat(270 * 1024); let stdoutListener: ((chunk: string) => void) | undefined; @@ -2552,8 +2722,10 @@ ${JSON.stringify({ expect(requireArgAfter(spawnArg.argv, "--permission-mode")).toBe("bypassPermissions"); }); - it("restarts Claude live sessions for env changes and fresh retries", async () => { + it("uses a fresh Claude live process and capture key for every captured turn", async () => { + const logWarnSpy = vi.spyOn(cliBackendLog, "warn").mockImplementation(() => undefined); const cancels: Array> = []; + const captureKeys: string[] = []; const turnResults = ["first-ok", "resume-ok", "env-ok", "fresh-ok"]; let turnIndex = 0; supervisorSpawnMock.mockImplementation(async (...args: unknown[]) => { @@ -2561,6 +2733,30 @@ ${JSON.stringify({ const input = (args[0] ?? {}) as { onStdout?: (chunk: string) => void }; const cancel = vi.fn(); cancels.push(cancel); + let resolveExit: (() => void) | undefined; + const exited = new Promise<{ + reason: "manual-cancel"; + exitCode: null; + exitSignal: null; + durationMs: number; + stdout: string; + stderr: string; + timedOut: false; + noOutputTimedOut: false; + }>((resolve) => { + resolveExit = () => + resolve({ + reason: "manual-cancel", + exitCode: null, + exitSignal: null, + durationMs: 1, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + cancel.mockImplementation(() => resolveExit?.()); return { runId: `live-run-${spawnIndex}`, pid: 2345 + spawnIndex, @@ -2583,7 +2779,7 @@ ${JSON.stringify({ }), end: vi.fn(), }, - wait: vi.fn(() => new Promise(() => {})), + wait: vi.fn(() => exited), cancel, }; }); @@ -2596,6 +2792,7 @@ ${JSON.stringify({ liveSession: "claude-stdio", resumeArgs: ["-p", "--output-format", "stream-json", "--resume", "{sessionId}"], }, + mcpDeliveryCapture: true, }); const result = await runClaudeLiveSessionTurn({ context, @@ -2613,7 +2810,12 @@ ${JSON.stringify({ getRecord: vi.fn(), }), onAssistantDelta: () => {}, - cleanup: async () => {}, + onMcpCaptureReady: (captureKey) => captureKeys.push(captureKey), + cleanup: async () => { + if (runId === "run-live-resume") { + throw new Error("captured cleanup failed"); + } + }, }); return result.output.text; }; @@ -2626,14 +2828,17 @@ ${JSON.stringify({ await expect( runTurn("run-live-resume", resumeArgs, { ANTHROPIC_BASE_URL: "https://one.example" }), ).resolves.toBe("resume-ok"); - expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); - expect(cancels[0]).not.toHaveBeenCalled(); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(2); + expect(cancels[0]).toHaveBeenCalledWith("manual-cancel"); + expect(cancels[1]).toHaveBeenCalledWith("manual-cancel"); + expect(captureKeys[1]).not.toBe(captureKeys[0]); await expect( runTurn("run-live-env-change", resumeArgs, { ANTHROPIC_BASE_URL: "https://two.example" }), ).resolves.toBe("env-ok"); - expect(supervisorSpawnMock).toHaveBeenCalledTimes(2); - expect(cancels[0]).toHaveBeenCalledWith("manual-cancel"); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(3); + expect(cancels[2]).toHaveBeenCalledWith("manual-cancel"); + expect(captureKeys[2]).not.toBe(captureKeys[1]); await expect( runTurn("run-live-fresh-retry", freshArgs, { @@ -2641,9 +2846,12 @@ ${JSON.stringify({ }), ).resolves.toBe("fresh-ok"); - expect(supervisorSpawnMock).toHaveBeenCalledTimes(3); - expect(cancels[1]).toHaveBeenCalledWith("manual-cancel"); - expect(cancels[2]).not.toHaveBeenCalled(); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(4); + expect(cancels[3]).toHaveBeenCalledWith("manual-cancel"); + expect(captureKeys[3]).not.toBe(captureKeys[2]); + expect(logWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Claude live session cleanup failed: captured cleanup failed"), + ); }); it("ignores non-JSON stdout lines from Claude live sessions", async () => { diff --git a/src/agents/cli-runner.ts b/src/agents/cli-runner.ts index a11d8c3b3541..26a89edcc922 100644 --- a/src/agents/cli-runner.ts +++ b/src/agents/cli-runner.ts @@ -14,6 +14,11 @@ import { createSubsystemLogger } from "../logging/subsystem.js"; import { buildAgentHookContextChannelFields } from "../plugins/hook-agent-context.js"; import { resolveBlockMessage } from "../plugins/hook-decision-types.js"; import { getGlobalHookRunner } from "../plugins/hook-runner-global.js"; +import type { CliOutput } from "./cli-output.js"; +import { + attachCliMessagingDeliveryEvidence, + getCliMessagingDeliveryEvidence, +} from "./cli-runner/delivery-evidence.js"; import { cliBackendLog, formatCliBackendOutputDigest } from "./cli-runner/log.js"; import { loadCliSessionContextEngineMessages, @@ -23,6 +28,7 @@ import type { PreparedCliRunContext, RunCliAgentParams } from "./cli-runner/type import { claudeCliSessionTranscriptHasContent as claudeCliSessionTranscriptHasContentImpl } from "./command/attempt-execution.helpers.js"; import { classifyFailoverReason, isFailoverErrorMessage } from "./embedded-agent-helpers.js"; import type { EmbeddedAgentRunResult } from "./embedded-agent-runner.js"; +import { buildEmbeddedRunPayloads } from "./embedded-agent-runner/run/payloads.js"; import { FailoverError, isFailoverError, resolveFailoverStatus } from "./failover-error.js"; import { awaitAgentEndSideEffects, @@ -420,19 +426,46 @@ async function runCliAgentInternal(params: RunCliAgentParams): Promise { + cleanupError ??= error; + }; + if (params.cleanupCliLiveSessionOnRunEnd === true) { + try { const { closeClaudeLiveSessionForContext } = await import("./cli-runner/claude-live-session.js"); await closeClaudeLiveSessionForContext(context); - } - if (params.cleanupBundleMcpOnRunEnd === true) { - const { closeMcpLoopbackServer } = await import("../gateway/mcp-http.js"); - await closeMcpLoopbackServer(); + } catch (error) { + recordCleanupError(error); } } + if (params.cleanupBundleMcpOnRunEnd === true) { + try { + const { closeMcpLoopbackServer } = await import("../gateway/mcp-http.js"); + await closeMcpLoopbackServer(); + } catch (error) { + recordCleanupError(error); + } + } + if (cleanupError) { + if (runError || result?.didSendViaMessagingTool === true) { + log.warn(`cli run cleanup failed after completion: ${formatErrorMessage(cleanupError)}`); + } else { + runError = + cleanupError instanceof Error ? cleanupError : new Error(formatErrorMessage(cleanupError)); + } + } + if (runError) { + throw runError instanceof Error ? runError : new Error(formatErrorMessage(runError)); + } + return result as EmbeddedAgentRunResult; } /** Runs an already-prepared CLI agent context through hooks and execution. */ @@ -557,6 +590,115 @@ export async function runPreparedCliAgent( }, }); + let deliveredMessagingSideEffect = false; + const buildCliSourceReplyMirrorPayloads = ( + evidence: Pick< + CliOutput, + | "didSendViaMessagingTool" + | "didDeliverSourceReplyViaMessageTool" + | "messagingToolSourceReplyPayloads" + >, + ): ReplyPayload[] => { + return buildEmbeddedRunPayloads({ + assistantTexts: [], + toolMetas: [], + lastAssistant: undefined, + inlineToolResultsAllowed: false, + sessionKey: params.sessionKey ?? "", + provider: params.provider, + model: context.modelId, + didSendViaMessagingTool: evidence.didSendViaMessagingTool, + didDeliverSourceReplyViaMessageTool: evidence.didDeliverSourceReplyViaMessageTool, + messagingToolSourceReplyPayloads: evidence.messagingToolSourceReplyPayloads, + sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, + agentId: params.agentId, + runId: params.runId, + }); + }; + + const resolveCliSourceReplyMirror = ( + evidence: Pick< + CliOutput, + | "didSendViaMessagingTool" + | "didDeliverSourceReplyViaMessageTool" + | "messagingToolSourceReplyPayloads" + >, + ) => { + const payloads = buildCliSourceReplyMirrorPayloads(evidence); + const delivered = + payloads.length > 0 || + (params.sourceReplyDeliveryMode === "message_tool_only" && + evidence.didDeliverSourceReplyViaMessageTool === true); + const visibleText = + payloads + .map((payload) => payload.text?.trim() ?? "") + .filter(Boolean) + .join("\n\n") || undefined; + return { payloads, delivered, visibleText }; + }; + + const buildDeliveredFailureResult = ( + error: unknown, + evidence: NonNullable>, + ): EmbeddedAgentRunResult => { + const message = formatErrorMessage(error); + const { payloads } = resolveCliSourceReplyMirror(evidence); + deliveredMessagingSideEffect = true; + return { + ...(payloads.length > 0 ? { payloads } : {}), + meta: { + durationMs: Date.now() - context.started, + systemPromptReport: context.systemPromptReport, + stopReason: "error", + executionTrace: { + winnerProvider: params.provider, + winnerModel: context.modelId, + attempts: [ + { + provider: params.provider, + model: context.modelId, + result: "error", + reason: message, + }, + ], + fallbackUsed: false, + runner: "cli", + }, + requestShaping: { + ...(params.thinkLevel ? { thinking: params.thinkLevel } : {}), + ...(context.effectiveAuthProfileId ? { authMode: "auth-profile" } : {}), + }, + completion: { + finishReason: "error", + stopReason: "error", + refusal: false, + }, + agentMeta: { + sessionId: "", + provider: params.provider, + model: context.modelId, + ...(context.reusableCliSession.sessionId ? { clearCliSessionBinding: true } : {}), + }, + }, + didSendViaMessagingTool: true, + ...(evidence.didDeliverSourceReplyViaMessageTool + ? { didDeliverSourceReplyViaMessageTool: true } + : {}), + ...(evidence.messagingToolSentTexts?.length + ? { messagingToolSentTexts: evidence.messagingToolSentTexts } + : {}), + ...(evidence.messagingToolSentMediaUrls?.length + ? { messagingToolSentMediaUrls: evidence.messagingToolSentMediaUrls } + : {}), + ...(evidence.messagingToolSentTargets?.length + ? { messagingToolSentTargets: evidence.messagingToolSentTargets } + : {}), + ...(evidence.messagingToolSourceReplyPayloads?.length + ? { messagingToolSourceReplyPayloads: evidence.messagingToolSourceReplyPayloads } + : {}), + }; + }; + const persistBlockedBeforeAgentRun = async (block: { message: string; pluginId: string; @@ -618,15 +760,25 @@ export async function runPreparedCliAgent( }, }; const output = await executePreparedCliRun(attemptContext, cliSessionIdToUse); - const assistantText = output.text.trim(); - if (!assistantText && params.allowEmptyAssistantReplyAsSilent !== true) { - throw new FailoverError("CLI backend returned an empty response.", { - reason: "empty_response", - provider: params.provider, - model: context.modelId, - sessionId: params.sessionId, - lane: params.lane, - }); + const sourceReplyMirror = resolveCliSourceReplyMirror(output); + const assistantText = sourceReplyMirror.delivered + ? (sourceReplyMirror.visibleText ?? "") + : output.text.trim(); + if ( + !assistantText && + !output.didSendViaMessagingTool && + params.allowEmptyAssistantReplyAsSilent !== true + ) { + throw attachCliMessagingDeliveryEvidence( + new FailoverError("CLI backend returned an empty response.", { + reason: "empty_response", + provider: params.provider, + model: context.modelId, + sessionId: params.sessionId, + lane: params.lane, + }), + output, + ); } const assistantTexts = assistantText ? [assistantText] : []; const lastAssistant = @@ -663,7 +815,12 @@ export async function runPreparedCliAgent( hookRunner, }); } - return { output, assistantText, lastAssistant }; + return { + output, + assistantText, + lastAssistant, + sourceReplyWasDelivered: sourceReplyMirror.delivered, + }; }; const buildCliRunResult = (resultParams: { @@ -674,15 +831,27 @@ export async function runPreparedCliAgent( }): EmbeddedAgentRunResult => { const text = resultParams.output.text?.trim(); const rawText = resultParams.output.rawText?.trim(); - const payloads = text - ? [ - resultParams.assistantTranscriptOwned - ? setReplyPayloadMetadata({ text }, { assistantTranscriptOwned: true }) - : { text }, - ] - : params.allowEmptyAssistantReplyAsSilent === true - ? [{ text: SILENT_REPLY_TOKEN }] - : undefined; + const sourceReplyMirror = resolveCliSourceReplyMirror(resultParams.output); + const finalAssistantVisibleText = sourceReplyMirror.delivered + ? sourceReplyMirror.visibleText + : text; + const payloads = + sourceReplyMirror.payloads.length > 0 + ? sourceReplyMirror.payloads + : sourceReplyMirror.delivered + ? undefined + : text + ? [ + resultParams.assistantTranscriptOwned + ? setReplyPayloadMetadata({ text }, { assistantTranscriptOwned: true }) + : { text }, + ] + : params.allowEmptyAssistantReplyAsSilent === true + ? [{ text: SILENT_REPLY_TOKEN }] + : undefined; + if (resultParams.output.didSendViaMessagingTool) { + deliveredMessagingSideEffect = true; + } const unflushedCliSessionId = resultParams.effectiveCliSessionId && resultParams.bindingFlushOk === false ? resultParams.effectiveCliSessionId @@ -701,9 +870,9 @@ export async function runPreparedCliAgent( ...(resultParams.output.finalPromptText ? { finalPromptText: resultParams.output.finalPromptText } : {}), - ...(text || rawText + ...(finalAssistantVisibleText || rawText ? { - ...(text ? { finalAssistantVisibleText: text } : {}), + ...(finalAssistantVisibleText ? { finalAssistantVisibleText } : {}), ...(rawText ? { finalAssistantRawText: rawText } : {}), } : {}), @@ -748,6 +917,9 @@ export async function runPreparedCliAgent( ...(context.extraSystemPromptHash ? { extraSystemPromptHash: context.extraSystemPromptHash } : {}), + ...(context.messageToolPolicyHash + ? { messageToolPolicyHash: context.messageToolPolicyHash } + : {}), ...(context.promptToolNamesHash ? { promptToolNamesHash: context.promptToolNamesHash } : {}), @@ -764,10 +936,26 @@ export async function runPreparedCliAgent( ...(unflushedCliSessionId ? { clearCliSessionBinding: true } : {}), }, }, + ...(resultParams.output.didSendViaMessagingTool ? { didSendViaMessagingTool: true } : {}), + ...(resultParams.output.didDeliverSourceReplyViaMessageTool + ? { didDeliverSourceReplyViaMessageTool: true } + : {}), + ...(resultParams.output.messagingToolSentTexts?.length + ? { messagingToolSentTexts: resultParams.output.messagingToolSentTexts } + : {}), + ...(resultParams.output.messagingToolSentMediaUrls?.length + ? { messagingToolSentMediaUrls: resultParams.output.messagingToolSentMediaUrls } + : {}), + ...(resultParams.output.messagingToolSentTargets?.length + ? { messagingToolSentTargets: resultParams.output.messagingToolSentTargets } + : {}), + ...(resultParams.output.messagingToolSourceReplyPayloads?.length + ? { messagingToolSourceReplyPayloads: resultParams.output.messagingToolSourceReplyPayloads } + : {}), }; }; - try { + const executeRun = async (): Promise => { await bootstrapHarnessContextEngine({ hadSessionFile: context.hadSessionFile, contextEngine: context.contextEngine, @@ -790,41 +978,61 @@ export async function runPreparedCliAgent( result: Awaited>, fallbackCliSessionId?: string, ) => { - const { output, lastAssistant } = result; - const assistantText = output.text.trim(); - const effectiveCliSessionId = output.sessionId ?? fallbackCliSessionId; - await finalizeCliContextEngineTurn({ - context, - historyMessages: context.contextEngine ? contextEngineHistoryMessages : historyMessages, - assistantText, - output, - }); - const assistantTranscriptOwned = await persistCliAssistantTranscript({ - runParams: params, - text: assistantText, - modelId: context.modelId, - usage: output.usage, - }); - const bindingFlushOk = await isCliBindingFlushed( - effectiveCliSessionId, - params.provider, - context.cwd ?? context.workspaceDir, - ); + const { output, assistantText, lastAssistant, sourceReplyWasDelivered } = result; + try { + const effectiveCliSessionId = output.sessionId ?? fallbackCliSessionId; + await finalizeCliContextEngineTurn({ + context, + historyMessages: context.contextEngine ? contextEngineHistoryMessages : historyMessages, + assistantText, + output, + }); + const assistantTranscriptOwned = await persistCliAssistantTranscript({ + runParams: params, + // Dispatch owns source-reply transcript mirrors and their idempotency keys. + // Persisting them here would duplicate the same visible assistant reply. + text: sourceReplyWasDelivered ? "" : assistantText, + modelId: context.modelId, + usage: output.usage, + }); + const bindingFlushOk = await isCliBindingFlushed( + effectiveCliSessionId, + params.provider, + context.cwd ?? context.workspaceDir, + ); + await runCliAgentEndHook(params, { + event: { + messages: buildAgentEndMessages(lastAssistant), + success: true, + durationMs: Date.now() - context.started, + }, + ctx: hookContext, + hookRunner, + }); + return buildCliRunResult({ + output, + effectiveCliSessionId, + bindingFlushOk, + assistantTranscriptOwned, + }); + } catch (error) { + throw attachCliMessagingDeliveryEvidence(error, output); + } + }; + + const finishDeliveredFailure = async ( + error: unknown, + ): Promise => { + const evidence = getCliMessagingDeliveryEvidence(error); + if (!evidence) { + return undefined; + } await runCliAgentEndHook(params, { - event: { - messages: buildAgentEndMessages(lastAssistant), - success: true, - durationMs: Date.now() - context.started, - }, + event: buildFailedAgentEndEvent(formatErrorMessage(error)), ctx: hookContext, hookRunner, }); - return buildCliRunResult({ - output, - effectiveCliSessionId, - bindingFlushOk, - assistantTranscriptOwned, - }); + return buildDeliveredFailureResult(error, evidence); }; if (hasBeforeAgentRunHooks && hookRunner) { @@ -894,6 +1102,10 @@ export async function runPreparedCliAgent( context.reusableCliSession.sessionId, ); } catch (err) { + const deliveredFailure = await finishDeliveredFailure(err); + if (deliveredFailure) { + return deliveredFailure; + } if (isFailoverError(err)) { const retryableSessionId = context.reusableCliSession.sessionId; if ( @@ -924,6 +1136,10 @@ export async function runPreparedCliAgent( ); return await finishCliAttempt(await executeCliAttempt(undefined, retryTimeoutMs)); } catch (retryErr) { + const deliveredRetryFailure = await finishDeliveredFailure(retryErr); + if (deliveredRetryFailure) { + return deliveredRetryFailure; + } const retryMessage = formatErrorMessage(retryErr); await runCliAgentEndHook(params, { event: buildFailedAgentEndEvent(retryMessage), @@ -948,9 +1164,39 @@ export async function runPreparedCliAgent( }); return toCliRunFailure(err); } - } finally { - await context.preparedBackend.cleanup?.(); + }; + + let runResult: EmbeddedAgentRunResult | undefined; + let runError: unknown; + let runFailed = false; + try { + runResult = await executeRun(); + } catch (error) { + runFailed = true; + runError = error; } + try { + await context.preparedBackend.cleanup?.(); + } catch (cleanupError) { + if (!deliveredMessagingSideEffect) { + if (runFailed) { + cliBackendLog.warn( + `CLI run also failed before backend cleanup: ${formatErrorMessage(runError)}`, + ); + } + throw cleanupError; + } + cliBackendLog.warn( + `CLI backend cleanup failed after confirmed message delivery: ${formatErrorMessage(cleanupError)}`, + ); + } + if (runFailed) { + throw runError; + } + if (!runResult) { + throw new Error("CLI run completed without a result"); + } + return runResult; } /** Legacy Claude-specific wrapper params for the generic CLI runner. */ @@ -985,6 +1231,7 @@ export function buildRunClaudeCliAgentParams(params: RunClaudeCliAgentParams): R extraSystemPrompt: params.extraSystemPrompt, inputProvenance: params.inputProvenance, sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, + requireExplicitMessageTarget: params.requireExplicitMessageTarget, silentReplyPromptMode: params.silentReplyPromptMode, extraSystemPromptStatic: params.extraSystemPromptStatic, ownerNumbers: params.ownerNumbers, diff --git a/src/agents/cli-runner/bundle-mcp-gemini.ts b/src/agents/cli-runner/bundle-mcp-gemini.ts index f1841d0d0a06..bb4adc5a2b1a 100644 --- a/src/agents/cli-runner/bundle-mcp-gemini.ts +++ b/src/agents/cli-runner/bundle-mcp-gemini.ts @@ -101,3 +101,46 @@ export async function writeGeminiSystemSettings( }, }; } + +/** Writes per-attempt Gemini settings with the active loopback capture token. */ +export async function writeGeminiMcpCaptureSettings(params: { + inheritedEnv: Record | undefined; + captureKey: string; +}): Promise<{ env: Record; cleanup: () => Promise }> { + const existingSettingsPath = params.inheritedEnv?.GEMINI_CLI_SYSTEM_SETTINGS_PATH; + if (!existingSettingsPath) { + throw new Error("Gemini MCP capture requires prepared system settings"); + } + const settings = await readJsonObject(existingSettingsPath); + const mcpServers = isRecord(settings.mcpServers) ? settings.mcpServers : {}; + const openclaw = isRecord(mcpServers.openclaw) ? mcpServers.openclaw : {}; + const headers = normalizeStringRecord(openclaw.headers) ?? {}; + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-gemini-mcp-attempt-")); + const settingsPath = path.join(tempDir, "settings.json"); + await writeJson( + settingsPath, + { + ...settings, + mcpServers: { + ...mcpServers, + openclaw: { + ...openclaw, + headers: { + ...headers, + "x-openclaw-cli-capture-key": params.captureKey, + }, + }, + }, + }, + { trailingNewline: true }, + ); + return { + env: { + ...params.inheritedEnv, + GEMINI_CLI_SYSTEM_SETTINGS_PATH: settingsPath, + }, + cleanup: async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }, + }; +} diff --git a/src/agents/cli-runner/bundle-mcp.codex.test.ts b/src/agents/cli-runner/bundle-mcp.codex.test.ts index bd373ec13f79..1035ff02ea42 100644 --- a/src/agents/cli-runner/bundle-mcp.codex.test.ts +++ b/src/agents/cli-runner/bundle-mcp.codex.test.ts @@ -22,6 +22,7 @@ describe("prepareCliBundleMcpConfig codex", () => { headers: { Authorization: "Bearer ${OPENCLAW_MCP_TOKEN}", "x-session-key": "${OPENCLAW_MCP_SESSION_KEY}", + "x-openclaw-cli-capture-key": "${OPENCLAW_MCP_CLI_CAPTURE_KEY}", }, }, }, @@ -34,14 +35,14 @@ describe("prepareCliBundleMcpConfig codex", () => { "exec", "--json", "-c", - 'mcp_servers={ openclaw = { url = "http://127.0.0.1:23119/mcp", default_tools_approval_mode = "approve", bearer_token_env_var = "OPENCLAW_MCP_TOKEN", env_http_headers = { x-session-key = "OPENCLAW_MCP_SESSION_KEY" } } }', + 'mcp_servers={ openclaw = { url = "http://127.0.0.1:23119/mcp", default_tools_approval_mode = "approve", bearer_token_env_var = "OPENCLAW_MCP_TOKEN", env_http_headers = { x-session-key = "OPENCLAW_MCP_SESSION_KEY", x-openclaw-cli-capture-key = "OPENCLAW_MCP_CLI_CAPTURE_KEY" } } }', ]); expect(prepared.backend.resumeArgs).toEqual([ "exec", "resume", "{sessionId}", "-c", - 'mcp_servers={ openclaw = { url = "http://127.0.0.1:23119/mcp", default_tools_approval_mode = "approve", bearer_token_env_var = "OPENCLAW_MCP_TOKEN", env_http_headers = { x-session-key = "OPENCLAW_MCP_SESSION_KEY" } } }', + 'mcp_servers={ openclaw = { url = "http://127.0.0.1:23119/mcp", default_tools_approval_mode = "approve", bearer_token_env_var = "OPENCLAW_MCP_TOKEN", env_http_headers = { x-session-key = "OPENCLAW_MCP_SESSION_KEY", x-openclaw-cli-capture-key = "OPENCLAW_MCP_CLI_CAPTURE_KEY" } } }', ]); expect(prepared.cleanup).toBeUndefined(); }); diff --git a/src/agents/cli-runner/bundle-mcp.gemini.test.ts b/src/agents/cli-runner/bundle-mcp.gemini.test.ts index 632c87a54521..c8d5add790b4 100644 --- a/src/agents/cli-runner/bundle-mcp.gemini.test.ts +++ b/src/agents/cli-runner/bundle-mcp.gemini.test.ts @@ -1,7 +1,7 @@ /** Tests Gemini CLI bundle-MCP system settings generation. */ import fs from "node:fs/promises"; import { describe, expect, it } from "vitest"; -import { prepareCliBundleMcpConfig } from "./bundle-mcp.js"; +import { prepareCliBundleMcpCaptureAttempt, prepareCliBundleMcpConfig } from "./bundle-mcp.js"; describe("prepareCliBundleMcpConfig gemini", () => { it("writes Gemini system settings for bundle MCP servers", async () => { @@ -95,4 +95,51 @@ describe("prepareCliBundleMcpConfig gemini", () => { await prepared.cleanup?.(); }); + + it("writes a unique capture token into per-attempt Gemini settings", async () => { + const prepared = await prepareCliBundleMcpConfig({ + enabled: true, + mode: "gemini-system-settings", + backend: { + command: "gemini", + args: ["--prompt", "{prompt}"], + }, + workspaceDir: "/tmp/openclaw-bundle-mcp-gemini", + config: { plugins: { enabled: false } }, + additionalConfig: { + mcpServers: { + openclaw: { + type: "http", + url: "http://127.0.0.1:23119/mcp", + headers: { + "x-openclaw-cli-capture-key": "${OPENCLAW_MCP_CLI_CAPTURE_KEY}", + }, + }, + }, + }, + env: { + OPENCLAW_MCP_CLI_CAPTURE_KEY: "", + }, + }); + const attempt = await prepareCliBundleMcpCaptureAttempt({ + mode: "gemini-system-settings", + env: prepared.env, + captureKey: "attempt-123", + }); + + try { + const raw = JSON.parse( + await fs.readFile(attempt.env?.GEMINI_CLI_SYSTEM_SETTINGS_PATH as string, "utf-8"), + ) as { + mcpServers?: Record }>; + }; + expect(raw.mcpServers?.openclaw?.headers?.["x-openclaw-cli-capture-key"]).toBe("attempt-123"); + expect(attempt.env?.GEMINI_CLI_SYSTEM_SETTINGS_PATH).not.toBe( + prepared.env?.GEMINI_CLI_SYSTEM_SETTINGS_PATH, + ); + } finally { + await attempt.cleanup?.(); + await prepared.cleanup?.(); + } + }); }); diff --git a/src/agents/cli-runner/bundle-mcp.ts b/src/agents/cli-runner/bundle-mcp.ts index bb7d6e89575f..18ee0defdc3c 100644 --- a/src/agents/cli-runner/bundle-mcp.ts +++ b/src/agents/cli-runner/bundle-mcp.ts @@ -15,7 +15,7 @@ import { loadMergedBundleMcpConfig, toCliBundleMcpServerConfig } from "../bundle import { isRecord } from "./bundle-mcp-adapter-shared.js"; import { findClaudeMcpConfigPath, injectClaudeMcpConfigArgs } from "./bundle-mcp-claude.js"; import { injectCodexMcpConfigArgs } from "./bundle-mcp-codex.js"; -import { writeGeminiSystemSettings } from "./bundle-mcp-gemini.js"; +import { writeGeminiMcpCaptureSettings, writeGeminiSystemSettings } from "./bundle-mcp-gemini.js"; type PreparedCliBundleMcpConfig = { backend: CliBackendConfig; @@ -197,3 +197,26 @@ export async function prepareCliBundleMcpConfig(params: { env: params.env, }); } + +/** Prepares a per-attempt capture token without changing resume compatibility hashes. */ +export async function prepareCliBundleMcpCaptureAttempt(params: { + mode?: CliBundleMcpMode; + env?: Record; + captureKey?: string; +}): Promise<{ env?: Record; cleanup?: () => Promise }> { + if (!params.captureKey) { + return { env: params.env }; + } + if (resolveBundleMcpMode(params.mode) === "gemini-system-settings") { + return await writeGeminiMcpCaptureSettings({ + inheritedEnv: params.env, + captureKey: params.captureKey, + }); + } + return { + env: { + ...params.env, + OPENCLAW_MCP_CLI_CAPTURE_KEY: params.captureKey, + }, + }; +} diff --git a/src/agents/cli-runner/claude-live-session.ts b/src/agents/cli-runner/claude-live-session.ts index 7818607107c2..c9e9f6d91934 100644 --- a/src/agents/cli-runner/claude-live-session.ts +++ b/src/agents/cli-runner/claude-live-session.ts @@ -11,6 +11,7 @@ import { type DiagnosticToolExecutionErrorEvent, type DiagnosticToolExecutionCompletedEvent, } from "../../infra/diagnostic-events.js"; +import { formatErrorMessage } from "../../infra/errors.js"; import { loadExecApprovals, maxAsk, @@ -71,8 +72,9 @@ type ClaudeLiveSession = { drainingAbortedTurn: boolean; idleTimer: NodeJS.Timeout | null; cleanup: () => Promise; - cleanupDone: boolean; + cleanupPromise: Promise | null; closing: boolean; + mcpCaptureKey?: string; }; type ClaudeLiveRunResult = { output: CliOutput; @@ -163,6 +165,13 @@ export async function closeClaudeLiveSessionForContext( liveSessionCreates.delete(key); } +/** Close a tainted live process so its replacement gets a fresh MCP capture key. */ +export async function rotateClaudeLiveMcpCaptureKeyForContext( + context: PreparedCliRunContext, +): Promise { + await closeClaudeLiveSessionForContext(context); +} + /** Returns whether a prepared backend context is eligible for Claude live stdio reuse. */ export function shouldUseClaudeLiveSession(context: PreparedCliRunContext): boolean { return ( @@ -418,12 +427,13 @@ function abortTurn(session: ClaudeLiveSession, error: Error): void { closeLiveSession(session, "abort", error); } -function cleanupLiveSession(session: ClaudeLiveSession): void { - if (session.cleanupDone) { - return; +function cleanupLiveSession(session: ClaudeLiveSession): Promise { + if (!session.cleanupPromise) { + session.cleanupPromise = session.cleanup().catch((error: unknown) => { + cliBackendLog.warn(`Claude live session cleanup failed: ${formatErrorMessage(error)}`); + }); } - session.cleanupDone = true; - void session.cleanup(); + return session.cleanupPromise; } function closeLiveSession( @@ -450,7 +460,7 @@ function closeLiveSession( failTurn(session, error); } session.managedRun.cancel("manual-cancel"); - cleanupLiveSession(session); + void cleanupLiveSession(session); } function scheduleIdleClose(session: ClaudeLiveSession): void { @@ -976,7 +986,7 @@ function handleClaudeExit(session: ClaudeLiveSession, exitCode: number | null): if (liveSessions.get(session.key) === session) { liveSessions.delete(session.key); } - cleanupLiveSession(session); + void cleanupLiveSession(session); if (!session.currentTurn) { return; } @@ -1047,6 +1057,7 @@ async function createClaudeLiveSession(params: { env: Record; fingerprint: string; key: string; + mcpCaptureKey?: string; noOutputTimeoutMs: number; supervisor: ProcessSupervisor; cleanup: () => Promise; @@ -1060,7 +1071,9 @@ async function createClaudeLiveSession(params: { mode: "child", argv: params.argv, cwd: params.context.cwd ?? params.context.workspaceDir, - env: params.env, + env: params.mcpCaptureKey + ? { ...params.env, OPENCLAW_MCP_CLI_CAPTURE_KEY: params.mcpCaptureKey } + : params.env, stdinMode: "pipe-open", captureOutput: false, onStdout: (chunk) => { @@ -1097,8 +1110,9 @@ async function createClaudeLiveSession(params: { drainingAbortedTurn: false, idleTimer: null, cleanup: params.cleanup, - cleanupDone: false, + cleanupPromise: null, closing: false, + mcpCaptureKey: params.mcpCaptureKey, }; void managedRun.wait().then( (exit) => handleClaudeExit(session, exit.exitCode), @@ -1220,6 +1234,7 @@ export async function runClaudeLiveSessionTurn(params: { onToolUseStart?: (delta: CliToolUseStartDelta) => void; onToolResult?: (delta: CliToolResultDelta) => void; onCommentaryText?: (text: string) => void; + onMcpCaptureReady?: (captureKey: string) => void; cleanup: () => Promise; }): Promise { const key = buildClaudeLiveKey(params.context); @@ -1292,6 +1307,7 @@ export async function runClaudeLiveSessionTurn(params: { env: params.env, fingerprint, key, + mcpCaptureKey: params.context.mcpDeliveryCapture ? crypto.randomUUID() : undefined, noOutputTimeoutMs: params.noOutputTimeoutMs, supervisor: params.getProcessSupervisor(), cleanup, @@ -1327,6 +1343,9 @@ export async function runClaudeLiveSessionTurn(params: { throw new Error("Claude CLI live session is already handling a turn"); } const liveSession = session; + if (liveSession.mcpCaptureKey) { + params.onMcpCaptureReady?.(liveSession.mcpCaptureKey); + } liveSession.noOutputTimeoutMs = params.noOutputTimeoutMs; liveSession.stderr = ""; @@ -1371,8 +1390,18 @@ export async function runClaudeLiveSessionTurn(params: { } finally { replyBackendCompleted = true; params.context.params.abortSignal?.removeEventListener("abort", abort); - if (replyBackendHandle) { - params.context.params.replyOperation?.detachBackend(replyBackendHandle); + try { + if (replyBackendHandle) { + params.context.params.replyOperation?.detachBackend(replyBackendHandle); + } + } finally { + if (liveSession.mcpCaptureKey) { + // The capture key is process environment, so a captured turn must end its + // process before the attempt releases that key to avoid cross-turn sends. + closeLiveSession(liveSession, "restart"); + await waitForManagedRunExit(liveSession.managedRun); + await cleanupLiveSession(liveSession); + } } } } diff --git a/src/agents/cli-runner/delivery-evidence.ts b/src/agents/cli-runner/delivery-evidence.ts new file mode 100644 index 000000000000..324b880a8cfb --- /dev/null +++ b/src/agents/cli-runner/delivery-evidence.ts @@ -0,0 +1,79 @@ +/** + * Carries confirmed CLI messaging delivery across failed execution/finalization paths. + */ +import type { CliOutput } from "../cli-output.js"; + +const CLI_MESSAGING_DELIVERY_EVIDENCE_KEY = "cliMessagingDeliveryEvidence"; + +type CliMessagingDeliveryEvidence = Pick< + CliOutput, + | "didSendViaMessagingTool" + | "didDeliverSourceReplyViaMessageTool" + | "messagingToolSentTexts" + | "messagingToolSentMediaUrls" + | "messagingToolSentTargets" + | "messagingToolSourceReplyPayloads" +>; + +function snapshotCliMessagingDeliveryEvidence( + output: CliMessagingDeliveryEvidence, +): CliMessagingDeliveryEvidence | undefined { + if (output.didSendViaMessagingTool !== true) { + return undefined; + } + return { + didSendViaMessagingTool: true, + ...(output.didDeliverSourceReplyViaMessageTool + ? { didDeliverSourceReplyViaMessageTool: true } + : {}), + ...(output.messagingToolSentTexts?.length + ? { messagingToolSentTexts: output.messagingToolSentTexts.slice() } + : {}), + ...(output.messagingToolSentMediaUrls?.length + ? { messagingToolSentMediaUrls: output.messagingToolSentMediaUrls.slice() } + : {}), + ...(output.messagingToolSentTargets?.length + ? { messagingToolSentTargets: output.messagingToolSentTargets.slice() } + : {}), + ...(output.messagingToolSourceReplyPayloads?.length + ? { messagingToolSourceReplyPayloads: output.messagingToolSourceReplyPayloads.slice() } + : {}), + }; +} + +/** Attaches confirmed delivery evidence so caller retries cannot duplicate a visible send. */ +export function attachCliMessagingDeliveryEvidence( + error: unknown, + output: CliMessagingDeliveryEvidence, +): unknown { + const evidence = snapshotCliMessagingDeliveryEvidence(output); + if (!evidence) { + return error; + } + if (error && typeof error === "object") { + try { + Object.assign(error, { [CLI_MESSAGING_DELIVERY_EVIDENCE_KEY]: evidence }); + return error; + } catch { + // Frozen and non-extensible failures need a mutable wrapper. + } + } + const wrapped = new Error(error instanceof Error ? error.message : String(error), { + cause: error, + }); + Object.assign(wrapped, { [CLI_MESSAGING_DELIVERY_EVIDENCE_KEY]: evidence }); + return wrapped; +} + +/** Reads confirmed delivery evidence from a failed CLI attempt. */ +export function getCliMessagingDeliveryEvidence( + error: unknown, +): CliMessagingDeliveryEvidence | undefined { + if (!error || typeof error !== "object") { + return undefined; + } + const evidence = (error as Record)[CLI_MESSAGING_DELIVERY_EVIDENCE_KEY]; + return evidence && typeof evidence === "object" + ? snapshotCliMessagingDeliveryEvidence(evidence as CliMessagingDeliveryEvidence) + : undefined; +} diff --git a/src/agents/cli-runner/execute.supervisor-capture.test.ts b/src/agents/cli-runner/execute.supervisor-capture.test.ts index b0ba05dd600f..7e942d272a4e 100644 --- a/src/agents/cli-runner/execute.supervisor-capture.test.ts +++ b/src/agents/cli-runner/execute.supervisor-capture.test.ts @@ -1,15 +1,42 @@ // Covers CLI execution paths where the process supervisor keeps stdout capture // disabled and the runner must parse streamed chunks without relying on tails. import { beforeEach, describe, expect, it } from "vitest"; +import { + markMcpLoopbackToolCallFinished, + markMcpLoopbackToolCallStarted, + recordMcpLoopbackToolCallResult as recordMcpLoopbackToolCallResultForHandle, +} from "../../gateway/mcp-http.loopback-runtime.js"; import { onAgentEvent, resetAgentEventsForTest } from "../../infra/agent-events.js"; import type { getProcessSupervisor } from "../../process/supervisor/index.js"; import { createManagedRun, supervisorSpawnMock } from "../cli-runner.test-support.js"; +import { getCliMessagingDeliveryEvidence } from "./delivery-evidence.js"; import { executePreparedCliRun } from "./execute.js"; import type { PreparedCliRunContext } from "./types.js"; type ProcessSupervisor = ReturnType; type SupervisorSpawnInput = Parameters[0]; +function recordMcpLoopbackToolCallResult(params: { + captureKey: string; + toolName: string; + args: Record; + result?: unknown; + isError: boolean; +}): void { + const captureHandle = markMcpLoopbackToolCallStarted(params); + if (!captureHandle) { + return; + } + recordMcpLoopbackToolCallResultForHandle({ + captureHandle, + toolName: params.toolName, + args: params.args, + result: params.result, + isError: params.isError, + }); + markMcpLoopbackToolCallFinished(captureHandle); +} + function buildPreparedCliRunContext(params: { output: "jsonl" | "text"; provider?: string; @@ -313,4 +340,737 @@ describe("executePreparedCliRun supervisor output capture", () => { stop(); } }); + + it("reports only confirmed message deliveries from correlated JSONL tool events", async () => { + const chunks = [ + `${JSON.stringify({ + type: "assistant", + message: { + role: "assistant", + content: [ + { + type: "mcp_tool_use", + id: "message-send-1", + name: "mcp__openclaw__message", + input: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + }, + }, + { + type: "mcp_tool_result", + tool_use_id: "message-send-1", + content: [{ type: "text", text: JSON.stringify({ result: { messageId: "msg-1" } }) }], + }, + ], + }, + })}\n`, + `${JSON.stringify({ type: "result", session_id: "session-jsonl", result: "done" })}\n`, + ]; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + }, + result: { ok: true, to: "spaces/AAA" }, + isError: false, + }); + for (const chunk of chunks) { + input.onStdout?.(chunk); + } + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const context = buildPreparedCliRunContext({ output: "jsonl", provider: "claude-cli" }); + context.mcpDeliveryCapture = true; + const result = await executePreparedCliRun(context); + + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.messagingToolSentTargets).toEqual([ + expect.objectContaining({ + tool: "message", + provider: "telegram", + to: "chat123", + text: "done", + }), + ]); + }); + + it("captures message text aliases from correlated JSONL tool events", async () => { + const chunks = [ + `${JSON.stringify({ + type: "assistant", + message: { + role: "assistant", + content: [ + { + type: "mcp_tool_use", + id: "message-send-text-alias", + name: "mcp__openclaw__message", + input: { + action: "send", + channel: "telegram", + target: "chat123", + text: "done", + }, + }, + { + type: "mcp_tool_result", + tool_use_id: "message-send-text-alias", + content: [{ type: "text", text: JSON.stringify({ status: "sent" }) }], + }, + ], + }, + })}\n`, + `${JSON.stringify({ type: "result", session_id: "session-jsonl", result: "done" })}\n`, + ]; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + for (const chunk of chunks) { + input.onStdout?.(chunk); + } + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun( + buildPreparedCliRunContext({ output: "jsonl", provider: "claude-cli" }), + ); + + expect(result.messagingToolSentTexts).toEqual(["done"]); + expect(result.messagingToolSentTargets).toEqual([ + expect.objectContaining({ + tool: "message", + provider: "telegram", + to: "chat123", + text: "done", + }), + ]); + }); + + it("bounds pending and committed JSONL message delivery evidence", async () => { + const starts = Array.from({ length: 65 }, (_, index) => ({ + type: "mcp_tool_use", + id: `message-send-${index}`, + name: "mcp__openclaw__message", + input: { + action: "send", + channel: "telegram", + target: `chat${index}`, + message: "done", + }, + })); + const results = starts.map((start) => ({ + type: "mcp_tool_result", + tool_use_id: start.id, + content: [{ type: "text", text: JSON.stringify({ status: "sent" }) }], + })); + const chunks = [ + `${JSON.stringify({ + type: "assistant", + message: { role: "assistant", content: [...starts, ...results] }, + })}\n`, + `${JSON.stringify({ type: "result", session_id: "session-jsonl", result: "done" })}\n`, + ]; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + for (const chunk of chunks) { + input.onStdout?.(chunk); + } + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun( + buildPreparedCliRunContext({ output: "jsonl", provider: "claude-cli" }), + ); + + expect(result.messagingToolSentTargets).toHaveLength(64); + expect(result.messagingToolSentTargets?.[0]?.to).toBe("chat1"); + expect(result.messagingToolSentTargets?.at(-1)?.to).toBe("chat64"); + }); + + it("fails closed when an unresolved JSONL message send is evicted", async () => { + const starts = Array.from({ length: 65 }, (_, index) => ({ + type: "mcp_tool_use", + id: `message-send-${index}`, + name: "mcp__openclaw__message", + input: { + action: "send", + channel: "telegram", + target: `chat${index}`, + message: "done", + }, + })); + const chunks = [ + `${JSON.stringify({ + type: "assistant", + message: { + role: "assistant", + content: [ + ...starts, + { + type: "mcp_tool_result", + tool_use_id: starts[0]?.id, + content: [{ type: "text", text: JSON.stringify({ status: "sent" }) }], + }, + ], + }, + })}\n`, + ]; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + for (const chunk of chunks) { + input.onStdout?.(chunk); + } + return createManagedRun({ + reason: "exit", + exitCode: 1, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "failed", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + let thrown: unknown; + try { + await executePreparedCliRun( + buildPreparedCliRunContext({ output: "jsonl", provider: "claude-cli" }), + ); + } catch (error) { + thrown = error; + } + + expect(getCliMessagingDeliveryEvidence(thrown)?.didSendViaMessagingTool).toBe(true); + }); + + it("fails closed when a JSONL message send remains unresolved after exit", async () => { + const chunk = `${JSON.stringify({ + type: "assistant", + message: { + role: "assistant", + content: [ + { + type: "mcp_tool_use", + id: "message-send-unresolved", + name: "mcp__openclaw__message", + input: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + }, + }, + ], + }, + })}\n`; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + input.onStdout?.(chunk); + return createManagedRun({ + reason: "exit", + exitCode: 1, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "failed", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + let thrown: unknown; + try { + await executePreparedCliRun( + buildPreparedCliRunContext({ output: "jsonl", provider: "claude-cli" }), + ); + } catch (error) { + thrown = error; + } + + expect(getCliMessagingDeliveryEvidence(thrown)?.didSendViaMessagingTool).toBe(true); + }); + + it("keeps an unresolved JSONL dry-run message retryable", async () => { + const chunk = `${JSON.stringify({ + type: "assistant", + message: { + role: "assistant", + content: [ + { + type: "mcp_tool_use", + id: "message-dry-run-unresolved", + name: "mcp__openclaw__message", + input: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + dryRun: true, + }, + }, + ], + }, + })}\n`; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + input.onStdout?.(chunk); + return createManagedRun({ + reason: "exit", + exitCode: 1, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "failed", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + let thrown: unknown; + try { + await executePreparedCliRun( + buildPreparedCliRunContext({ output: "jsonl", provider: "claude-cli" }), + ); + } catch (error) { + thrown = error; + } + + expect(getCliMessagingDeliveryEvidence(thrown)?.didSendViaMessagingTool).toBeUndefined(); + }); + + it("fails closed for suppressed non-streaming MCP message results", async () => { + const context = buildPreparedCliRunContext({ output: "text", provider: "google-gemini-cli" }); + context.mcpDeliveryCapture = true; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + }, + result: { status: "suppressed" }, + isError: false, + }); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun(context); + + expect(result.didSendViaMessagingTool).toBeUndefined(); + expect(result.messagingToolSentTargets).toBeUndefined(); + }); + + it("keeps mutation delivery out of sent-reply dedupe evidence", async () => { + const context = buildPreparedCliRunContext({ output: "text", provider: "google-gemini-cli" }); + context.mcpDeliveryCapture = true; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "edit", + channel: "telegram", + target: "chat123", + message: "done", + }, + result: { ok: true }, + isError: false, + }); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun(context); + + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.messagingToolSentTexts).toBeUndefined(); + expect(result.messagingToolSentTargets).toBeUndefined(); + }); + + it("preserves the current provider for implicit message send targets", async () => { + const context = buildPreparedCliRunContext({ output: "text", provider: "google-gemini-cli" }); + context.mcpDeliveryCapture = true; + context.params.messageChannel = "slack"; + context.params.currentChannelId = "C123"; + context.params.currentThreadTs = "1700000000.000100"; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + target: "C123", + message: "done", + }, + result: { status: "sent" }, + isError: false, + }); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun(context); + + expect(result.messagingToolSentTargets).toEqual([ + expect.objectContaining({ + provider: "slack", + to: "C123", + }), + ]); + }); + + it("preserves partial delivery evidence from failed MCP message calls", async () => { + const context = buildPreparedCliRunContext({ output: "text", provider: "google-gemini-cli" }); + context.mcpDeliveryCapture = true; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + mediaUrl: "https://example.com/photo.png", + }, + result: Object.assign(new Error("second chunk failed"), { sentBeforeError: true }), + isError: true, + }); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun(context); + + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.messagingToolSentTargets).toEqual([ + expect.objectContaining({ + tool: "message", + provider: "telegram", + to: "chat123", + text: "done", + mediaUrls: ["https://example.com/photo.png"], + }), + ]); + }); + + it("reports confirmed non-streaming MCP message results from the serialized capture", async () => { + const context = buildPreparedCliRunContext({ output: "text", provider: "google-gemini-cli" }); + context.mcpDeliveryCapture = true; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + }, + result: { result: { messageId: "msg-1" } }, + isError: false, + }); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun(context); + + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.messagingToolSentTargets).toEqual([ + expect.objectContaining({ + tool: "message", + provider: "telegram", + to: "chat123", + text: "done", + }), + ]); + }); + + it("reports confirmed poll delivery from the serialized capture", async () => { + const context = buildPreparedCliRunContext({ output: "text", provider: "google-gemini-cli" }); + context.mcpDeliveryCapture = true; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "poll", + channel: "telegram", + target: "chat123", + pollQuestion: "Lunch?", + pollOption: ["Pizza", "Sushi"], + }, + result: { pollId: "poll-1" }, + isError: false, + }); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun(context); + + expect(result.didSendViaMessagingTool).toBe(true); + }); + + it("preserves text and media evidence for confirmed implicit message sends", async () => { + const context = buildPreparedCliRunContext({ output: "text", provider: "google-gemini-cli" }); + context.mcpDeliveryCapture = true; + context.params.sourceReplyDeliveryMode = "message_tool_only"; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + message: "implicit reply", + mediaUrl: "https://example.com/implicit.png", + }, + result: { + ok: true, + details: { + deliveryStatus: "sent", + sourceReplySink: "internal-ui", + sourceReply: { + text: "implicit reply", + mediaUrl: "https://example.com/implicit.png", + }, + }, + }, + isError: false, + }); + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + message: "implicit reply", + mediaUrl: "https://example.com/implicit.png", + }, + result: { + ok: true, + details: { + deliveryStatus: "sent", + sourceReplySink: "internal-ui", + sourceReply: { + text: "implicit reply", + mediaUrl: "https://example.com/implicit.png", + }, + }, + }, + isError: false, + }); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun(context); + + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.messagingToolSentTexts).toEqual(["implicit reply"]); + expect(result.messagingToolSentMediaUrls).toEqual(["https://example.com/implicit.png"]); + expect(result.messagingToolSentTargets).toBeUndefined(); + expect(result.didDeliverSourceReplyViaMessageTool).toBe(true); + expect(result.messagingToolSourceReplyPayloads).toEqual([ + { + text: "implicit reply", + mediaUrl: "https://example.com/implicit.png", + }, + { + text: "implicit reply", + mediaUrl: "https://example.com/implicit.png", + }, + ]); + }); + + it("retains confirmed delivery for long non-streaming message calls", async () => { + const context = buildPreparedCliRunContext({ output: "text", provider: "local-cli" }); + context.mcpDeliveryCapture = true; + supervisorSpawnMock.mockImplementationOnce(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + recordMcpLoopbackToolCallResult({ + captureKey: input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? "", + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "x".repeat(20 * 1024), + }, + result: { status: "sent" }, + isError: false, + }); + input.onStdout?.("done"); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const result = await executePreparedCliRun(context); + + expect(result.didSendViaMessagingTool).toBe(true); + expect(result.messagingToolSentTargets).toEqual([ + expect.objectContaining({ tool: "message", provider: "telegram", to: "chat123" }), + ]); + }); + + it("captures non-Claude JSONL sends and gives every attempt a unique token", async () => { + const context = buildPreparedCliRunContext({ output: "jsonl", provider: "local-cli" }); + context.mcpDeliveryCapture = true; + const captureKeys: string[] = []; + supervisorSpawnMock.mockImplementation(async (...args: unknown[]) => { + const input = args[0] as SupervisorSpawnInput; + const captureKey = input.env?.OPENCLAW_MCP_CLI_CAPTURE_KEY ?? ""; + captureKeys.push(captureKey); + recordMcpLoopbackToolCallResult({ + captureKey, + toolName: "message", + args: { + action: "send", + channel: "telegram", + target: "chat123", + message: "done", + }, + result: { status: "sent" }, + isError: false, + }); + input.onStdout?.(`${JSON.stringify({ item: { type: "message", text: "done" } })}\n`); + return createManagedRun({ + reason: "exit", + exitCode: 0, + exitSignal: null, + durationMs: 50, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }); + }); + + const first = await executePreparedCliRun(context); + const second = await executePreparedCliRun(context); + + expect(first.didSendViaMessagingTool).toBe(true); + expect(second.didSendViaMessagingTool).toBe(true); + expect(captureKeys).toHaveLength(2); + expect(captureKeys[0]).not.toBe(captureKeys[1]); + }); }); diff --git a/src/agents/cli-runner/execute.ts b/src/agents/cli-runner/execute.ts index 1d58504088e4..e09050938011 100644 --- a/src/agents/cli-runner/execute.ts +++ b/src/agents/cli-runner/execute.ts @@ -3,12 +3,19 @@ * live-session routing, and diagnostics. */ import crypto from "node:crypto"; +import { + beginMcpLoopbackToolCallCapture, + clearMcpLoopbackToolCallCapture, + type McpLoopbackToolCallStart, + waitForMcpLoopbackToolCallCaptureIdle, +} from "../../gateway/mcp-http.loopback-runtime.js"; import { shouldLogVerbose } from "../../globals.js"; import { assertAgentRunLifecycleGenerationCurrent, emitAgentEvent, } from "../../infra/agent-events.js"; import { isTruthyEnvValue } from "../../infra/env.js"; +import { formatErrorMessage } from "../../infra/errors.js"; import { resolveEventSessionKeyForPolicy, resolveEventSessionRoutingPolicy, @@ -16,6 +23,7 @@ import { } from "../../infra/event-session-routing.js"; import { requestHeartbeat as requestHeartbeatImpl } from "../../infra/heartbeat-wake.js"; import { sanitizeHostExecEnv } from "../../infra/host-env-security.js"; +import { shouldUseInternalSourceReplySink } from "../../infra/outbound/internal-source-reply.js"; import { enqueueSystemEvent as enqueueSystemEventImpl } from "../../infra/system-events.js"; import { getProcessSupervisor as getProcessSupervisorImpl } from "../../process/supervisor/index.js"; import { applySkillEnvOverridesFromSnapshot } from "../../skills/runtime/env-overrides.js"; @@ -27,11 +35,38 @@ import { type CliOutput, } from "../cli-output.js"; import { classifyFailoverReason } from "../embedded-agent-helpers.js"; -import { sanitizeToolArgs, sanitizeToolResult } from "../embedded-agent-subscribe.tools.js"; +import { + isDeliveredMessageToolOnlySourceReplyResult, + isDeliveredMessagingToolResult, +} from "../embedded-agent-message-tool-source-reply.js"; +import { + isMessagingTool, + isMessagingToolDeliveryAction, + isMessagingToolSendAction, +} from "../embedded-agent-messaging.js"; +import type { + MessagingToolSend, + MessagingToolSourceReplyPayload, +} from "../embedded-agent-messaging.types.js"; +import { + collectMessagingMediaUrlsFromRecord, + collectMessagingMediaUrlsFromToolResult, + extractMessagingToolSend, + extractMessagingToolSendResult, + extractMessagingToolSourceReplyPayload, + sanitizeToolArgs, + sanitizeToolResult, +} from "../embedded-agent-subscribe.tools.js"; import { FailoverError, resolveFailoverStatus } from "../failover-error.js"; import { applyPluginTextReplacements } from "../plugin-text-transforms.js"; -import { runClaudeLiveSessionTurn, shouldUseClaudeLiveSession } from "./claude-live-session.js"; +import { prepareCliBundleMcpCaptureAttempt } from "./bundle-mcp.js"; +import { + rotateClaudeLiveMcpCaptureKeyForContext, + runClaudeLiveSessionTurn, + shouldUseClaudeLiveSession, +} from "./claude-live-session.js"; import { prepareClaudeCliSkillsPlugin } from "./claude-skills-plugin.js"; +import { attachCliMessagingDeliveryEvidence } from "./delivery-evidence.js"; import { buildCliSupervisorScopeKey, buildClaudeOwnerKey, @@ -58,10 +93,109 @@ const executeDeps = { getProcessSupervisor: getProcessSupervisorImpl, enqueueSystemEvent: enqueueSystemEventImpl, requestHeartbeat: requestHeartbeatImpl, + writeCliSystemPromptFile, }; const CLI_RUNNER_OUTPUT_TAIL_BYTES = 64 * 1024; const CLI_RUNNER_OUTPUT_PARSE_BYTES = 1024 * 1024; +const CLI_MESSAGING_EVIDENCE_MAX_CALLS = 64; +const CLI_MCP_DELIVERY_DRAIN_GRACE_MS = 5_000; +const CLI_MCP_REQUEST_ADMISSION_GRACE_MS = 250; +const OPENCLAW_MCP_TOOL_PREFIX = "mcp__openclaw__"; + +function normalizeCliMessagingToolName(toolName: string): string { + return toolName.startsWith(OPENCLAW_MCP_TOOL_PREFIX) + ? toolName.slice(OPENCLAW_MCP_TOOL_PREFIX.length) + : toolName; +} + +function extractCliMessagingTarget( + context: PreparedCliRunContext, + toolName: string, + args: Record, +): MessagingToolSend | undefined { + const normalizedToolName = normalizeCliMessagingToolName(toolName); + const currentProvider = context.params.messageChannel ?? context.params.messageProvider; + const hasExplicitProvider = + (typeof args.provider === "string" && args.provider.trim().length > 0) || + (typeof args.channel === "string" && args.channel.trim().length > 0); + const targetArgs = + normalizedToolName === "message" && currentProvider && !hasExplicitProvider + ? { ...args, provider: currentProvider } + : args; + if (!isMessagingToolSendAction(normalizedToolName, targetArgs)) { + return undefined; + } + return extractMessagingToolSend(normalizedToolName, targetArgs, { + config: context.params.config, + currentChannelId: context.params.currentChannelId, + currentThreadId: context.params.currentThreadTs, + currentMessageId: context.params.currentMessageId, + }); +} + +function buildMessagingToolSendEvidenceKey(send: MessagingToolSend): string { + return crypto + .createHash("sha256") + .update( + JSON.stringify([ + send.tool, + send.provider, + send.accountId, + send.to, + send.threadId, + send.threadImplicit, + send.threadSuppressed, + send.text, + send.mediaUrls, + ]), + ) + .digest("hex"); +} + +function buildCliMcpCaptureKey(context: PreparedCliRunContext): string | undefined { + if (!context.mcpDeliveryCapture) { + return undefined; + } + return crypto.randomUUID(); +} + +function extractCliMessagingContent( + args: Record, + result: unknown, +): Pick { + const text = ["message", "SendMessage", "content", "text", "caption"] + .map((key) => args[key]) + .find((value): value is string => typeof value === "string" && value.trim().length > 0); + const mediaUrls = [ + ...collectMessagingMediaUrlsFromRecord(args), + ...collectMessagingMediaUrlsFromToolResult(result), + ].filter((url, index, all) => all.indexOf(url) === index); + return { + ...(text ? { text } : {}), + ...(mediaUrls.length > 0 ? { mediaUrls } : {}), + }; +} + +function appendUniqueCliMessagingEvidence( + values: string[], + valueKeys: Set, + additions: readonly string[], +): void { + for (const addition of additions) { + if (!addition || valueKeys.has(addition)) { + continue; + } + if (values.length >= CLI_MESSAGING_EVIDENCE_MAX_CALLS) { + const removed = values.shift(); + if (removed) { + valueKeys.delete(removed); + } + } + values.push(addition); + valueKeys.add(addition); + } +} function appendCliOutputTail(tail: Buffer, chunk: string): Buffer { if (!chunk) { @@ -301,7 +435,7 @@ export async function executePreparedCliRun( }); const systemPromptFile = systemPromptArg && (!useResume || backend.systemPromptWhen === "always") - ? await writeCliSystemPromptFile({ + ? await executeDeps.writeCliSystemPromptFile({ backend, systemPrompt: systemPromptArg, }) @@ -373,23 +507,46 @@ export async function executePreparedCliRun( useResume, }); + const claudeOwnerKey = buildClaudeOwnerKey({ + agentAccountId: params.agentAccountId, + agentId: params.agentId, + authProfileId: context.effectiveAuthProfileId, + sessionId: params.sessionId, + sessionKey: params.sessionKey, + }); const queueKey = resolveCliRunQueueKey({ backendId: context.backendResolved.id, + liveSession: backend.liveSession, serialize: backend.serialize, runId: params.runId, workspaceDir: context.workspaceDir, cliSessionId: useResume ? resolvedSessionId : undefined, - ownerKey: buildClaudeOwnerKey({ - agentAccountId: params.agentAccountId, - agentId: params.agentId, - authProfileId: context.effectiveAuthProfileId, - sessionId: params.sessionId, - sessionKey: params.sessionKey, - }), + ownerKey: claudeOwnerKey, }); + let completedOutput: CliOutput | undefined; + let executionError: unknown; + const cleanupOuterResource = async (cleanup: (() => Promise) | undefined) => { + try { + await cleanup?.(); + } catch (error) { + if (completedOutput?.didSendViaMessagingTool === true) { + cliBackendLog.warn( + `CLI outer resource cleanup failed after confirmed message delivery: ${formatErrorMessage(error)}`, + ); + return; + } + if (executionError !== undefined) { + cliBackendLog.warn( + `CLI outer resource cleanup also failed after run error: ${formatErrorMessage(error)}`, + ); + return; + } + throw error; + } + }; try { - return await enqueueCliRun(queueKey, async () => { + completedOutput = await enqueueCliRun(queueKey, async () => { if (params.lifecycleGeneration) { assertAgentRunLifecycleGenerationCurrent(params.lifecycleGeneration); } @@ -400,6 +557,81 @@ export async function executePreparedCliRun( config: params.config, }) : undefined; + let gatewayCaptureKey: string | undefined; + let cleanupMcpCaptureAttempt: (() => Promise) | undefined; + let didSendViaMessagingTool = false; + let didDeliverSourceReplyViaMessageTool = false; + let inFlightUnclassifiedMcpRequests = 0; + let inFlightMessagingToolCalls = 0; + const inFlightPreparedMessagingCalls = new Set(); + const pendingMessagingCalls = new Map< + string, + { toolName: string; args: Record; target?: MessagingToolSend } + >(); + const messagingToolSentTexts: string[] = []; + const messagingToolSentTextKeys = new Set(); + const messagingToolSentMediaUrls: string[] = []; + const messagingToolSentMediaUrlKeys = new Set(); + const messagingToolSentTargets: MessagingToolSend[] = []; + const messagingToolSentTargetKeys = new Set(); + const messagingToolSourceReplyPayloads: MessagingToolSourceReplyPayload[] = []; + const isPreparedInternalSourceReply = async (call: McpLoopbackToolCallStart) => { + if ( + context.params.sourceReplyDeliveryMode !== "message_tool_only" || + normalizeCliMessagingToolName(call.toolName) !== "message" || + call.args.action !== "send" || + !context.params.config + ) { + return false; + } + return await shouldUseInternalSourceReplySink( + { + cfg: context.params.config, + action: "send", + sessionKey: context.params.sessionKey, + sourceReplyDeliveryMode: context.params.sourceReplyDeliveryMode, + toolContext: { + currentChannelProvider: + context.params.messageChannel ?? context.params.messageProvider, + currentChannelId: context.params.currentChannelId, + currentThreadTs: context.params.currentThreadTs, + currentMessageId: context.params.currentMessageId, + }, + }, + call.args, + ); + }; + let runOutput: CliOutput | undefined; + let runError: unknown; + let runFailed = false; + const recordRunError = (error: unknown) => { + if (runFailed) { + return; + } + runFailed = true; + runError = error; + }; + const withMessagingDeliveryEvidence = (output: CliOutput): CliOutput => { + return { + ...output, + ...(didSendViaMessagingTool ? { didSendViaMessagingTool: true } : {}), + ...(didDeliverSourceReplyViaMessageTool + ? { didDeliverSourceReplyViaMessageTool: true } + : {}), + ...(messagingToolSentTexts.length > 0 + ? { messagingToolSentTexts: messagingToolSentTexts.slice() } + : {}), + ...(messagingToolSentMediaUrls.length > 0 + ? { messagingToolSentMediaUrls: messagingToolSentMediaUrls.slice() } + : {}), + ...(messagingToolSentTargets.length > 0 + ? { messagingToolSentTargets: messagingToolSentTargets.slice() } + : {}), + ...(messagingToolSourceReplyPayloads.length > 0 + ? { messagingToolSourceReplyPayloads: messagingToolSourceReplyPayloads.slice() } + : {}), + }; + }; try { cliBackendLog.info( buildCliExecLogLine({ @@ -418,6 +650,17 @@ export async function executePreparedCliRun( const logOutputText = isTruthyEnvValue(process.env[CLI_BACKEND_LOG_OUTPUT_ENV]) || isTruthyEnvValue(process.env[LEGACY_CLAUDE_CLI_LOG_OUTPUT_ENV]); + const outputMode = useResume ? (backend.resumeOutput ?? backend.output) : backend.output; + const hasJsonlOutput = outputMode === "jsonl"; + const initialGatewayCaptureKey = shouldUseClaudeLiveSession(context) + ? undefined + : buildCliMcpCaptureKey(context); + const mcpCaptureAttempt = await prepareCliBundleMcpCaptureAttempt({ + mode: context.backendResolved.bundleMcpMode, + env: context.preparedBackend.env, + captureKey: initialGatewayCaptureKey, + }); + cleanupMcpCaptureAttempt = mcpCaptureAttempt.cleanup; const env = (() => { const next = sanitizeHostExecEnv({ baseEnv: process.env, @@ -440,7 +683,7 @@ export async function executePreparedCliRun( }), ); } - Object.assign(next, context.preparedBackend.env); + Object.assign(next, mcpCaptureAttempt.env); // Never mark Claude CLI as host-managed. That marker routes runs into // Anthropic's separate host-managed usage tier instead of normal CLI @@ -478,8 +721,150 @@ export async function executePreparedCliRun( useResume, trigger: params.trigger, }); - const outputMode = useResume ? (backend.resumeOutput ?? backend.output) : backend.output; - const hasJsonlOutput = outputMode === "jsonl"; + const commitMessagingToolResult = (paramsLocal: { + toolName: string; + target?: MessagingToolSend; + args?: Record; + result?: unknown; + isError?: boolean; + }) => { + if (!isDeliveredMessagingToolResult(paramsLocal)) { + return; + } + didSendViaMessagingTool = true; + const toolArgs = paramsLocal.args ?? {}; + if (!isMessagingToolSendAction(paramsLocal.toolName, toolArgs)) { + return; + } + const content = extractCliMessagingContent(toolArgs, paramsLocal.result); + appendUniqueCliMessagingEvidence( + messagingToolSentTexts, + messagingToolSentTextKeys, + content.text ? [content.text] : [], + ); + appendUniqueCliMessagingEvidence( + messagingToolSentMediaUrls, + messagingToolSentMediaUrlKeys, + content.mediaUrls ?? [], + ); + if ( + isDeliveredMessageToolOnlySourceReplyResult({ + sourceReplyDeliveryMode: context.params.sourceReplyDeliveryMode, + toolName: paramsLocal.toolName, + args: paramsLocal.args, + result: paramsLocal.result, + isError: paramsLocal.isError, + }) + ) { + didDeliverSourceReplyViaMessageTool = true; + const sourceReplyPayload = extractMessagingToolSourceReplyPayload(paramsLocal.result); + if (sourceReplyPayload) { + if (messagingToolSourceReplyPayloads.length >= CLI_MESSAGING_EVIDENCE_MAX_CALLS) { + messagingToolSourceReplyPayloads.shift(); + } + // Each internal source-reply send is a distinct delivery, even when + // two intentional sends have identical text or media. + messagingToolSourceReplyPayloads.push(sourceReplyPayload); + } + } + if (paramsLocal.target) { + const confirmedTarget = extractMessagingToolSendResult( + paramsLocal.target, + paramsLocal.result, + ); + const targetWithContent = { + ...confirmedTarget, + ...content, + }; + const evidenceKey = buildMessagingToolSendEvidenceKey(targetWithContent); + if (messagingToolSentTargetKeys.has(evidenceKey)) { + return; + } + if (messagingToolSentTargets.length >= CLI_MESSAGING_EVIDENCE_MAX_CALLS) { + const removed = messagingToolSentTargets.shift(); + if (removed) { + messagingToolSentTargetKeys.delete(buildMessagingToolSendEvidenceKey(removed)); + } + } + messagingToolSentTargets.push(targetWithContent); + messagingToolSentTargetKeys.add(evidenceKey); + } + }; + const beginGatewayCapture = (captureKey: string | undefined) => { + if (!captureKey) { + return; + } + if (gatewayCaptureKey === captureKey) { + return; + } + if (gatewayCaptureKey) { + throw new Error("CLI MCP capture key changed during an active attempt"); + } + gatewayCaptureKey = captureKey; + const isAdmittedPotentialMessagingDelivery = (toolName: string) => { + return isMessagingTool(normalizeCliMessagingToolName(toolName)); + }; + const isPreparedMessagingDelivery = ( + toolName: string, + toolArgs: Record, + ) => { + return ( + toolArgs.dryRun !== true && + isMessagingToolDeliveryAction(normalizeCliMessagingToolName(toolName), toolArgs) + ); + }; + beginMcpLoopbackToolCallCapture({ + captureKey: gatewayCaptureKey, + onRequestStart: () => { + inFlightUnclassifiedMcpRequests += 1; + }, + onRequestClassified: () => { + inFlightUnclassifiedMcpRequests = Math.max(0, inFlightUnclassifiedMcpRequests - 1); + }, + onToolCallStart: (call) => { + if (isAdmittedPotentialMessagingDelivery(call.toolName)) { + inFlightMessagingToolCalls += 1; + } + }, + onToolCallUpdate: ({ previous, current }) => { + inFlightPreparedMessagingCalls.delete(previous); + const wasMessagingSend = isAdmittedPotentialMessagingDelivery(previous.toolName); + const isMessagingSend = isPreparedMessagingDelivery(current.toolName, current.args); + if (wasMessagingSend !== isMessagingSend) { + inFlightMessagingToolCalls = Math.max( + 0, + inFlightMessagingToolCalls + (isMessagingSend ? 1 : -1), + ); + } + if (isMessagingSend) { + inFlightPreparedMessagingCalls.add(current); + } + }, + onToolCallFinish: (call, { prepared }) => { + const isMessagingSend = prepared + ? isPreparedMessagingDelivery(call.toolName, call.args) + : isAdmittedPotentialMessagingDelivery(call.toolName); + if (isMessagingSend) { + inFlightMessagingToolCalls = Math.max(0, inFlightMessagingToolCalls - 1); + } + inFlightPreparedMessagingCalls.delete(call); + }, + onToolCallResult: ({ toolName, args: toolArgs, result, isError }) => { + const normalizedToolName = normalizeCliMessagingToolName(toolName); + if (!isMessagingToolDeliveryAction(normalizedToolName, toolArgs)) { + return; + } + commitMessagingToolResult({ + toolName: normalizedToolName, + target: extractCliMessagingTarget(context, normalizedToolName, toolArgs), + args: toolArgs, + result, + isError, + }); + }, + }); + }; + beginGatewayCapture(initialGatewayCaptureKey); let observedCliActivity = false; const emitLiveEvents = params.executionMode !== "side-question"; const emitCliToolUseStart = (event: { @@ -488,6 +873,27 @@ export async function executePreparedCliRun( args: Record; }) => { observedCliActivity = true; + const toolName = normalizeCliMessagingToolName(event.name); + if ( + !gatewayCaptureKey && + event.args.dryRun !== true && + isMessagingToolDeliveryAction(toolName, event.args) + ) { + if (pendingMessagingCalls.size >= CLI_MESSAGING_EVIDENCE_MAX_CALLS) { + const oldestToolCallId = pendingMessagingCalls.keys().next().value; + if (oldestToolCallId !== undefined) { + pendingMessagingCalls.delete(oldestToolCallId); + // Once an unresolved send is evicted, its later result cannot be + // correlated. Fail closed so a failed turn cannot duplicate it. + didSendViaMessagingTool = true; + } + } + pendingMessagingCalls.set(event.toolCallId, { + toolName, + args: event.args, + target: extractCliMessagingTarget(context, toolName, event.args), + }); + } if (!emitLiveEvents) { return; } @@ -509,6 +915,17 @@ export async function executePreparedCliRun( result?: unknown; }) => { observedCliActivity = true; + const pending = pendingMessagingCalls.get(event.toolCallId); + if (pending) { + pendingMessagingCalls.delete(event.toolCallId); + commitMessagingToolResult({ + toolName: pending.toolName, + target: pending.target, + args: pending.args, + result: event.result, + isError: event.isError, + }); + } if (!emitLiveEvents) { return; } @@ -557,9 +974,14 @@ export async function executePreparedCliRun( model: context.modelId, backend: context.backendResolved.id, }); - fallbackClaudeSkillsPluginCleanupOwned = true; - const ownedPreparedBackendCleanup = context.preparedBackend.cleanup; - context.preparedBackend.cleanup = undefined; + const liveSessionOwnsRunArtifacts = context.mcpDeliveryCapture !== true; + fallbackClaudeSkillsPluginCleanupOwned = liveSessionOwnsRunArtifacts; + const ownedPreparedBackendCleanup = liveSessionOwnsRunArtifacts + ? context.preparedBackend.cleanup + : undefined; + if (liveSessionOwnsRunArtifacts) { + context.preparedBackend.cleanup = undefined; + } const liveResult = await runClaudeLiveSessionTurn({ context, args, @@ -596,16 +1018,19 @@ export async function executePreparedCliRun( emitLiveEvents && context.params.emitCommentaryText ? emitCliCommentaryText : undefined, - cleanup: async () => { - try { - await fallbackClaudeSkillsPlugin?.cleanup(); - } finally { - await ownedPreparedBackendCleanup?.(); - } - }, + onMcpCaptureReady: beginGatewayCapture, + cleanup: liveSessionOwnsRunArtifacts + ? async () => { + try { + await fallbackClaudeSkillsPlugin?.cleanup(); + } finally { + await ownedPreparedBackendCleanup?.(); + } + } + : async () => {}, }); const rawText = liveResult.output.text; - return { + runOutput = { ...liveResult.output, rawText, finalPromptText: prompt, @@ -614,305 +1039,403 @@ export async function executePreparedCliRun( context.backendResolved.textTransforms?.output, ), }; - } - const streamingParser = hasJsonlOutput - ? createCliJsonlStreamingParser({ + } else { + const streamingParser = hasJsonlOutput + ? createCliJsonlStreamingParser({ + backend, + providerId: context.backendResolved.id, + onAssistantDelta: ({ text, delta }) => { + if (text || delta) { + observedCliActivity = true; + } + if (!emitLiveEvents) { + return; + } + emitAgentEvent({ + runId: params.runId, + stream: "assistant", + data: { + text: applyPluginTextReplacements( + text, + context.backendResolved.textTransforms?.output, + ), + delta: applyPluginTextReplacements( + delta, + context.backendResolved.textTransforms?.output, + ), + }, + }); + }, + onToolUseStart: emitCliToolUseStart, + onToolResult: emitCliToolResult, + onCommentaryText: + emitLiveEvents && context.params.emitCommentaryText + ? emitCliCommentaryText + : undefined, + }) + : null; + const supervisor = executeDeps.getProcessSupervisor(); + const scopeKey = buildCliSupervisorScopeKey({ + backend, + backendId: context.backendResolved.id, + cliSessionId: useResume ? resolvedSessionId : undefined, + }); + let stdoutTail: Buffer = Buffer.alloc(0); + let stdoutParseBuffer: Buffer = Buffer.alloc(0); + let stdoutParseExceeded = false; + let stderrTail: Buffer = Buffer.alloc(0); + let stderrParseBuffer: Buffer = Buffer.alloc(0); + let stderrParseExceeded = false; + + params.onExecutionPhase?.({ + phase: "process_spawned", + provider: params.provider, + model: context.modelId, + backend: context.backendResolved.id, + }); + const managedRun = await supervisor.spawn({ + sessionId: params.sessionId, + backendId: context.backendResolved.id, + scopeKey, + replaceExistingScope: Boolean(useResume && scopeKey), + mode: "child", + argv: [backend.command, ...args], + timeoutMs: params.timeoutMs, + noOutputTimeoutMs, + cwd: context.cwd ?? context.workspaceDir, + env, + input: stdinPayload, + captureOutput: false, + onStdout: (chunk: string) => { + stdoutTail = appendCliOutputTail(stdoutTail, chunk); + if (!stdoutParseExceeded) { + const nextStdoutParse = appendCliOutputParseBuffer(stdoutParseBuffer, chunk); + stdoutParseBuffer = nextStdoutParse.buffer; + stdoutParseExceeded = nextStdoutParse.exceeded; + } + streamingParser?.push(chunk); + }, + onStderr: (chunk: string) => { + stderrTail = appendCliOutputTail(stderrTail, chunk); + if (!stderrParseExceeded) { + const nextStderrParse = appendCliOutputParseBuffer(stderrParseBuffer, chunk); + stderrParseBuffer = nextStderrParse.buffer; + stderrParseExceeded = nextStderrParse.exceeded; + } + }, + }); + let replyBackendCompleted = false; + const replyBackendHandle = params.replyOperation + ? { + kind: "cli" as const, + cancel: () => { + managedRun.cancel("manual-cancel"); + }, + isStreaming: () => !replyBackendCompleted, + } + : undefined; + if (replyBackendHandle) { + params.replyOperation?.attachBackend(replyBackendHandle); + } + const abortManagedRun = () => { + managedRun.cancel("manual-cancel"); + }; + params.abortSignal?.addEventListener("abort", abortManagedRun, { once: true }); + if (params.abortSignal?.aborted) { + abortManagedRun(); + } + let result: Awaited>; + try { + result = await managedRun.wait(); + } finally { + replyBackendCompleted = true; + if (replyBackendHandle) { + params.replyOperation?.detachBackend(replyBackendHandle); + } + params.abortSignal?.removeEventListener("abort", abortManagedRun); + } + streamingParser?.finish(); + if (params.abortSignal?.aborted && result.reason === "manual-cancel") { + throw createCliAbortError(); + } + + const stdout = stdoutParseBuffer.toString("utf8").trim(); + const stdoutDiagnostic = stdoutTail.toString("utf8").trim(); + const stderr = stderrParseBuffer.toString("utf8").trim(); + const stderrDiagnostic = stderrTail.toString("utf8").trim(); + if (logOutputText) { + if (stdoutDiagnostic) { + cliBackendLog.info(`cli stdout:\n${stdoutDiagnostic}`); + } + if (stderrDiagnostic) { + cliBackendLog.info(`cli stderr:\n${stderrDiagnostic}`); + } + } + if (shouldLogVerbose()) { + if (stdoutDiagnostic) { + cliBackendLog.debug(`cli stdout:\n${stdoutDiagnostic}`); + } + if (stderrDiagnostic) { + cliBackendLog.debug(`cli stderr:\n${stderrDiagnostic}`); + } + } + + if (result.exitCode !== 0 || result.reason !== "exit") { + if (result.reason === "no-output-timeout" || result.noOutputTimedOut) { + const timeoutReason = `CLI produced no output for ${Math.round(noOutputTimeoutMs / 1000)}s and was terminated.`; + cliBackendLog.warn( + `cli watchdog timeout: provider=${params.provider} model=${context.modelId} session=${resolvedSessionId ?? params.sessionId} noOutputTimeoutMs=${noOutputTimeoutMs} pid=${managedRun.pid ?? "unknown"}`, + ); + const retryableNoOutputTimeout = + !observedCliActivity && + stdoutDiagnostic.length === 0 && + stderrDiagnostic.length === 0; + const deferWatchdogNoticeForFreshRetry = + retryableNoOutputTimeout && + Boolean(cliSessionIdToUse) && + Boolean(resolvedSessionId) && + Boolean(context.openClawHistoryPrompt) && + Boolean(params.sessionKey) && + params.timeoutMs - (Date.now() - context.started) > 0; + if (params.sessionKey && emitLiveEvents && !deferWatchdogNoticeForFreshRetry) { + const stallNotice = [ + `CLI agent (${params.provider}) produced no output for ${Math.round(noOutputTimeoutMs / 1000)}s and was terminated.`, + "It may have been waiting for interactive input or an approval prompt.", + "For Claude Code, prefer --permission-mode bypassPermissions --print.", + ].join(" "); + const eventRouting = resolveEventSessionRoutingPolicy({ + cfg: params.config, + sessionKey: params.sessionKey, + channel: params.messageProvider, + accountId: params.agentAccountId, + }); + executeDeps.enqueueSystemEvent(stallNotice, { + sessionKey: resolveEventSessionKeyForPolicy(params.sessionKey, eventRouting), + }); + executeDeps.requestHeartbeat( + scopedHeartbeatWakeOptionsForPolicy( + params.sessionKey, + { + source: "cli-watchdog", + intent: "event", + reason: "cli:watchdog:stall", + }, + eventRouting, + ), + ); + } + throw new FailoverError(timeoutReason, { + reason: "timeout", + provider: params.provider, + model: context.modelId, + sessionId: params.sessionId, + lane: params.lane, + status: resolveFailoverStatus("timeout"), + code: retryableNoOutputTimeout ? "cli_no_output_timeout" : undefined, + }); + } + if (result.reason === "overall-timeout") { + const timeoutReason = `CLI exceeded timeout (${Math.round(params.timeoutMs / 1000)}s) and was terminated.`; + throw new FailoverError(timeoutReason, { + reason: "timeout", + provider: params.provider, + model: context.modelId, + sessionId: params.sessionId, + lane: params.lane, + status: resolveFailoverStatus("timeout"), + code: "cli_overall_timeout", + }); + } + const errorCandidates = [stderr, stdout, stderrDiagnostic, stdoutDiagnostic].filter( + (candidate) => candidate.length > 0, + ); + const structuredError = + errorCandidates.map((candidate) => extractCliErrorMessage(candidate)).find(Boolean) ?? + null; + let classifiedErrorText = structuredError; + let reason = structuredError + ? classifyFailoverReason(structuredError, { provider: params.provider }) + : null; + if (!reason) { + for (const candidate of errorCandidates) { + reason = classifyFailoverReason(candidate, { provider: params.provider }); + if (reason) { + classifiedErrorText = candidate; + break; + } + } + } + const err = + structuredError || classifiedErrorText || errorCandidates[0] || "CLI failed."; + reason = reason ?? "unknown"; + const status = resolveFailoverStatus(reason); + const retryCode = + reason === "unknown" && + result.reason === "exit" && + errorCandidates.length === 0 && + !observedCliActivity + ? "cli_unknown_empty_failure" + : undefined; + throw new FailoverError(err, { + reason, + provider: params.provider, + model: context.modelId, + sessionId: params.sessionId, + lane: params.lane, + status, + code: retryCode, + }); + } + + const streamedJsonlOutput = + outputMode === "jsonl" ? (streamingParser?.getOutput() ?? null) : null; + + if (stdoutParseExceeded && !streamedJsonlOutput) { + throw new FailoverError( + `CLI stdout exceeded ${CLI_RUNNER_OUTPUT_PARSE_BYTES} bytes; refusing to parse truncated output.`, + { + reason: "format", + provider: params.provider, + model: context.modelId, + sessionId: params.sessionId, + lane: params.lane, + status: resolveFailoverStatus("format"), + }, + ); + } + + const parsed = + streamedJsonlOutput ?? + parseCliOutput({ + raw: stdout, backend, providerId: context.backendResolved.id, - onAssistantDelta: ({ text, delta }) => { - if (text || delta) { - observedCliActivity = true; - } - if (!emitLiveEvents) { - return; - } - emitAgentEvent({ - runId: params.runId, - stream: "assistant", - data: { - text: applyPluginTextReplacements( - text, - context.backendResolved.textTransforms?.output, - ), - delta: applyPluginTextReplacements( - delta, - context.backendResolved.textTransforms?.output, - ), - }, - }); - }, - onToolUseStart: emitCliToolUseStart, - onToolResult: emitCliToolResult, - onCommentaryText: - emitLiveEvents && context.params.emitCommentaryText - ? emitCliCommentaryText - : undefined, - }) - : null; - const supervisor = executeDeps.getProcessSupervisor(); - const scopeKey = buildCliSupervisorScopeKey({ - backend, - backendId: context.backendResolved.id, - cliSessionId: useResume ? resolvedSessionId : undefined, - }); - let stdoutTail: Buffer = Buffer.alloc(0); - let stdoutParseBuffer: Buffer = Buffer.alloc(0); - let stdoutParseExceeded = false; - let stderrTail: Buffer = Buffer.alloc(0); - let stderrParseBuffer: Buffer = Buffer.alloc(0); - let stderrParseExceeded = false; - - params.onExecutionPhase?.({ - phase: "process_spawned", - provider: params.provider, - model: context.modelId, - backend: context.backendResolved.id, - }); - const managedRun = await supervisor.spawn({ - sessionId: params.sessionId, - backendId: context.backendResolved.id, - scopeKey, - replaceExistingScope: Boolean(useResume && scopeKey), - mode: "child", - argv: [backend.command, ...args], - timeoutMs: params.timeoutMs, - noOutputTimeoutMs, - cwd: context.cwd ?? context.workspaceDir, - env, - input: stdinPayload, - captureOutput: false, - onStdout: (chunk: string) => { - stdoutTail = appendCliOutputTail(stdoutTail, chunk); - if (!stdoutParseExceeded) { - const nextStdoutParse = appendCliOutputParseBuffer(stdoutParseBuffer, chunk); - stdoutParseBuffer = nextStdoutParse.buffer; - stdoutParseExceeded = nextStdoutParse.exceeded; - } - streamingParser?.push(chunk); - }, - onStderr: (chunk: string) => { - stderrTail = appendCliOutputTail(stderrTail, chunk); - if (!stderrParseExceeded) { - const nextStderrParse = appendCliOutputParseBuffer(stderrParseBuffer, chunk); - stderrParseBuffer = nextStderrParse.buffer; - stderrParseExceeded = nextStderrParse.exceeded; - } - }, - }); - let replyBackendCompleted = false; - const replyBackendHandle = params.replyOperation - ? { - kind: "cli" as const, - cancel: () => { - managedRun.cancel("manual-cancel"); - }, - isStreaming: () => !replyBackendCompleted, - } - : undefined; - if (replyBackendHandle) { - params.replyOperation?.attachBackend(replyBackendHandle); + outputMode, + fallbackSessionId: resolvedSessionId, + }); + const rawText = parsed.text; + cliBackendLog.info( + `cli turn: provider=${params.provider} model=${context.modelId} durationMs=${Date.now() - cliTurnStartedAt} ${formatCliBackendOutputDigest(rawText)}`, + ); + runOutput = { + ...parsed, + rawText, + finalPromptText: prompt, + text: applyPluginTextReplacements( + rawText, + context.backendResolved.textTransforms?.output, + ), + }; } - const abortManagedRun = () => { - managedRun.cancel("manual-cancel"); - }; - params.abortSignal?.addEventListener("abort", abortManagedRun, { once: true }); - if (params.abortSignal?.aborted) { - abortManagedRun(); - } - let result: Awaited>; + } catch (error) { + recordRunError(error); + } finally { try { - result = await managedRun.wait(); - } finally { - replyBackendCompleted = true; - if (replyBackendHandle) { - params.replyOperation?.detachBackend(replyBackendHandle); - } - params.abortSignal?.removeEventListener("abort", abortManagedRun); - } - streamingParser?.finish(); - if (params.abortSignal?.aborted && result.reason === "manual-cancel") { - throw createCliAbortError(); - } - - const stdout = stdoutParseBuffer.toString("utf8").trim(); - const stdoutDiagnostic = stdoutTail.toString("utf8").trim(); - const stderr = stderrParseBuffer.toString("utf8").trim(); - const stderrDiagnostic = stderrTail.toString("utf8").trim(); - if (logOutputText) { - if (stdoutDiagnostic) { - cliBackendLog.info(`cli stdout:\n${stdoutDiagnostic}`); - } - if (stderrDiagnostic) { - cliBackendLog.info(`cli stderr:\n${stderrDiagnostic}`); - } - } - if (shouldLogVerbose()) { - if (stdoutDiagnostic) { - cliBackendLog.debug(`cli stdout:\n${stdoutDiagnostic}`); - } - if (stderrDiagnostic) { - cliBackendLog.debug(`cli stderr:\n${stderrDiagnostic}`); - } - } - - if (result.exitCode !== 0 || result.reason !== "exit") { - if (result.reason === "no-output-timeout" || result.noOutputTimedOut) { - const timeoutReason = `CLI produced no output for ${Math.round(noOutputTimeoutMs / 1000)}s and was terminated.`; - cliBackendLog.warn( - `cli watchdog timeout: provider=${params.provider} model=${context.modelId} session=${resolvedSessionId ?? params.sessionId} noOutputTimeoutMs=${noOutputTimeoutMs} pid=${managedRun.pid ?? "unknown"}`, + if (!gatewayCaptureKey && pendingMessagingCalls.size > 0) { + const unresolvedJsonlMessagingCalls = Array.from(pendingMessagingCalls.values()); + const internalSourceReplyStates = await Promise.all( + unresolvedJsonlMessagingCalls.map(isPreparedInternalSourceReply), ); - const retryableNoOutputTimeout = - !observedCliActivity && - stdoutDiagnostic.length === 0 && - stderrDiagnostic.length === 0; - const deferWatchdogNoticeForFreshRetry = - retryableNoOutputTimeout && - Boolean(cliSessionIdToUse) && - Boolean(resolvedSessionId) && - Boolean(context.openClawHistoryPrompt) && - Boolean(params.sessionKey) && - params.timeoutMs - (Date.now() - context.started) > 0; - if (params.sessionKey && emitLiveEvents && !deferWatchdogNoticeForFreshRetry) { - const stallNotice = [ - `CLI agent (${params.provider}) produced no output for ${Math.round(noOutputTimeoutMs / 1000)}s and was terminated.`, - "It may have been waiting for interactive input or an approval prompt.", - "For Claude Code, prefer --permission-mode bypassPermissions --print.", - ].join(" "); - const eventRouting = resolveEventSessionRoutingPolicy({ - cfg: params.config, - sessionKey: params.sessionKey, - channel: params.messageProvider, - accountId: params.agentAccountId, - }); - executeDeps.enqueueSystemEvent(stallNotice, { - sessionKey: resolveEventSessionKeyForPolicy(params.sessionKey, eventRouting), - }); - executeDeps.requestHeartbeat( - scopedHeartbeatWakeOptionsForPolicy( - params.sessionKey, - { - source: "cli-watchdog", - intent: "event", - reason: "cli:watchdog:stall", - }, - eventRouting, - ), + const hasPotentialVisibleSend = internalSourceReplyStates.some( + (isInternalSourceReply) => !isInternalSourceReply, + ); + if (hasPotentialVisibleSend) { + // A JSONL start without a result may have delivered before the CLI exited. + // Fail closed so retry/failover cannot duplicate a late visible send. + didSendViaMessagingTool = true; + recordRunError( + new Error("CLI JSONL message tool call remained unresolved after exit"), + ); + } else { + recordRunError( + new Error("CLI JSONL source reply call remained unresolved after exit"), ); } - throw new FailoverError(timeoutReason, { - reason: "timeout", - provider: params.provider, - model: context.modelId, - sessionId: params.sessionId, - lane: params.lane, - status: resolveFailoverStatus("timeout"), - code: retryableNoOutputTimeout ? "cli_no_output_timeout" : undefined, - }); } - if (result.reason === "overall-timeout") { - const timeoutReason = `CLI exceeded timeout (${Math.round(params.timeoutMs / 1000)}s) and was terminated.`; - throw new FailoverError(timeoutReason, { - reason: "timeout", - provider: params.provider, - model: context.modelId, - sessionId: params.sessionId, - lane: params.lane, - status: resolveFailoverStatus("timeout"), - code: "cli_overall_timeout", - }); - } - const errorCandidates = [stderr, stdout, stderrDiagnostic, stdoutDiagnostic].filter( - (candidate) => candidate.length > 0, - ); - const structuredError = - errorCandidates.map((candidate) => extractCliErrorMessage(candidate)).find(Boolean) ?? - null; - let classifiedErrorText = structuredError; - let reason = structuredError - ? classifyFailoverReason(structuredError, { provider: params.provider }) - : null; - if (!reason) { - for (const candidate of errorCandidates) { - reason = classifyFailoverReason(candidate, { provider: params.provider }); - if (reason) { - classifiedErrorText = candidate; - break; + if (gatewayCaptureKey) { + const captureBecameIdle = await waitForMcpLoopbackToolCallCaptureIdle( + gatewayCaptureKey, + { + timeoutMs: CLI_MCP_DELIVERY_DRAIN_GRACE_MS, + admissionGraceMs: CLI_MCP_REQUEST_ADMISSION_GRACE_MS, + }, + ); + if (!captureBecameIdle) { + if (shouldUseClaudeLiveSession(context)) { + await rotateClaudeLiveMcpCaptureKeyForContext(context); + } + const unresolvedPreparedMessagingCalls = Array.from(inFlightPreparedMessagingCalls); + const internalSourceReplyStates = await Promise.all( + unresolvedPreparedMessagingCalls.map(isPreparedInternalSourceReply), + ); + const internalSourceReplyCount = internalSourceReplyStates.filter(Boolean).length; + const hasPotentialVisibleSend = inFlightMessagingToolCalls > internalSourceReplyCount; + if (inFlightUnclassifiedMcpRequests > 0 || hasPotentialVisibleSend) { + // An admitted request or send may complete after its CLI process exits. + // Fail closed so retry/failover cannot duplicate a late visible send. + didSendViaMessagingTool = true; + recordRunError(new Error("CLI message tool call remained in flight after exit")); + } else if (inFlightMessagingToolCalls > 0) { + // Internal source replies are only result payloads; they have no external + // side effect, so keep the failed turn retryable instead of dropping them. + recordRunError(new Error("CLI source reply call remained in flight after exit")); } } } - const err = structuredError || classifiedErrorText || errorCandidates[0] || "CLI failed."; - reason = reason ?? "unknown"; - const status = resolveFailoverStatus(reason); - const retryCode = - reason === "unknown" && - result.reason === "exit" && - errorCandidates.length === 0 && - !observedCliActivity - ? "cli_unknown_empty_failure" - : undefined; - throw new FailoverError(err, { - reason, - provider: params.provider, - model: context.modelId, - sessionId: params.sessionId, - lane: params.lane, - status, - code: retryCode, - }); + } catch (error) { + if ( + pendingMessagingCalls.size > 0 || + inFlightUnclassifiedMcpRequests > 0 || + inFlightMessagingToolCalls > 0 + ) { + // A failed drain/classification cannot prove an admitted messaging request harmless. + didSendViaMessagingTool = true; + } + recordRunError(error); + } finally { + if (gatewayCaptureKey) { + clearMcpLoopbackToolCallCapture(gatewayCaptureKey); + } } - - const streamedJsonlOutput = - outputMode === "jsonl" ? (streamingParser?.getOutput() ?? null) : null; - - if (stdoutParseExceeded && !streamedJsonlOutput) { - throw new FailoverError( - `CLI stdout exceeded ${CLI_RUNNER_OUTPUT_PARSE_BYTES} bytes; refusing to parse truncated output.`, - { - reason: "format", - provider: params.provider, - model: context.modelId, - sessionId: params.sessionId, - lane: params.lane, - status: resolveFailoverStatus("format"), - }, - ); + try { + await cleanupMcpCaptureAttempt?.(); + } catch (error) { + recordRunError(error); + } + try { + restoreSkillEnv?.(); + } catch (error) { + recordRunError(error); } - - const parsed = - streamedJsonlOutput ?? - parseCliOutput({ - raw: stdout, - backend, - providerId: context.backendResolved.id, - outputMode, - fallbackSessionId: resolvedSessionId, - }); - const rawText = parsed.text; - cliBackendLog.info( - `cli turn: provider=${params.provider} model=${context.modelId} durationMs=${Date.now() - cliTurnStartedAt} ${formatCliBackendOutputDigest(rawText)}`, - ); - return { - ...parsed, - rawText, - finalPromptText: prompt, - text: applyPluginTextReplacements( - rawText, - context.backendResolved.textTransforms?.output, - ), - }; - } finally { - restoreSkillEnv?.(); } + if (runFailed) { + throw attachCliMessagingDeliveryEvidence(runError, { + didSendViaMessagingTool, + didDeliverSourceReplyViaMessageTool, + messagingToolSentTexts, + messagingToolSentMediaUrls, + messagingToolSentTargets, + messagingToolSourceReplyPayloads, + }); + } + if (!runOutput) { + throw new Error("CLI run completed without output"); + } + return withMessagingDeliveryEvidence(runOutput); }); + return completedOutput; + } catch (error) { + executionError = error; + throw error; } finally { if (!fallbackClaudeSkillsPluginCleanupOwned) { - await fallbackClaudeSkillsPlugin?.cleanup(); + await cleanupOuterResource(fallbackClaudeSkillsPlugin?.cleanup); } if (systemPromptFile) { - await systemPromptFile.cleanup(); + await cleanupOuterResource(systemPromptFile.cleanup); } if (cleanupImages) { - await cleanupImages(); + await cleanupOuterResource(cleanupImages); } } } diff --git a/src/agents/cli-runner/helpers.system-prompt.test.ts b/src/agents/cli-runner/helpers.system-prompt.test.ts index b12b54b69556..bc7c1a51f6a2 100644 --- a/src/agents/cli-runner/helpers.system-prompt.test.ts +++ b/src/agents/cli-runner/helpers.system-prompt.test.ts @@ -124,4 +124,17 @@ describe("buildCliAgentSystemPrompt", () => { expect(prompt).toContain("channel=telegram"); expect(prompt).not.toContain("### message tool"); }); + + it("requires an explicit message target when the CLI turn policy requires one", () => { + const prompt = buildCliAgentSystemPrompt({ + workspaceDir: "/tmp/openclaw", + tools: [{ name: "message" } as never], + modelDisplay: "test/model", + sourceReplyDeliveryMode: "message_tool_only", + requireExplicitMessageTarget: true, + }); + + expect(prompt).toContain("include `target` and `message`; `target` is required for this turn"); + expect(prompt).not.toContain("The target defaults to the current source channel"); + }); }); diff --git a/src/agents/cli-runner/helpers.ts b/src/agents/cli-runner/helpers.ts index ec1e10bf8548..8a99241ab7ff 100644 --- a/src/agents/cli-runner/helpers.ts +++ b/src/agents/cli-runner/helpers.ts @@ -85,21 +85,27 @@ export function buildClaudeOwnerKey(input: { /** Resolves the serialization key for a CLI backend run. */ export function resolveCliRunQueueKey(params: { backendId: string; + liveSession?: CliBackendConfig["liveSession"]; serialize?: boolean; runId: string; workspaceDir: string; cliSessionId?: string; ownerKey?: string; }): string { - if (params.serialize === false) { + const requiresLiveSessionSerialization = + isClaudeCliProvider(params.backendId) && params.liveSession === "claude-stdio"; + if (params.serialize === false && !requiresLiveSessionSerialization) { return `${params.backendId}:${params.runId}`; } if (isClaudeCliProvider(params.backendId)) { + const ownerKey = params.ownerKey?.trim(); + if (requiresLiveSessionSerialization && ownerKey) { + return `${params.backendId}:owner:${ownerKey}`; + } const sessionId = params.cliSessionId?.trim(); if (sessionId) { return `${params.backendId}:session:${sessionId}`; } - const ownerKey = params.ownerKey?.trim(); if (ownerKey) { return `${params.backendId}:owner:${ownerKey}`; } @@ -119,6 +125,7 @@ export function buildCliAgentSystemPrompt(params: { defaultThinkLevel?: ThinkLevel; extraSystemPrompt?: string; sourceReplyDeliveryMode?: SourceReplyDeliveryMode; + requireExplicitMessageTarget?: boolean; silentReplyPromptMode?: SilentReplyPromptMode; runtimeChannel?: string; runtimeChatType?: ChatType; @@ -168,6 +175,7 @@ export function buildCliAgentSystemPrompt(params: { defaultThinkLevel: params.defaultThinkLevel, extraSystemPrompt: params.extraSystemPrompt, sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, + requireExplicitMessageTarget: params.requireExplicitMessageTarget, silentReplyPromptMode: params.silentReplyPromptMode, ownerNumbers: params.ownerNumbers, reasoningTagHint: false, diff --git a/src/agents/cli-runner/prepare.test.ts b/src/agents/cli-runner/prepare.test.ts index 5f46d237249d..5604a22e0c85 100644 --- a/src/agents/cli-runner/prepare.test.ts +++ b/src/agents/cli-runner/prepare.test.ts @@ -128,6 +128,7 @@ function createTestMcpLoopbackServerConfig(port: number) { "x-openclaw-source-reply-delivery-mode": "${OPENCLAW_MCP_SOURCE_REPLY_DELIVERY_MODE}", "x-openclaw-require-explicit-message-target": "${OPENCLAW_MCP_REQUIRE_EXPLICIT_MESSAGE_TARGET}", + "x-openclaw-cli-capture-key": "${OPENCLAW_MCP_CLI_CAPTURE_KEY}", }, }, }, @@ -1099,6 +1100,70 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { } }); + it("invalidates CLI session reuse when explicit message-target policy changes", async () => { + const { dir, sessionFile } = createSessionFile(); + try { + const context = await prepareCliRunContext({ + sessionId: "session-test", + sessionFile, + workspaceDir: dir, + prompt: "latest ask", + provider: "test-cli", + model: "test-model", + timeoutMs: 1_000, + runId: "run-test-message-policy", + sourceReplyDeliveryMode: "message_tool_only", + requireExplicitMessageTarget: true, + cliSessionBinding: { + sessionId: "cli-session", + messageToolPolicyHash: hashCliSessionText( + JSON.stringify({ + sourceReplyDeliveryMode: "message_tool_only", + requireExplicitMessageTarget: false, + }), + ), + }, + config: createCliBackendConfig(), + }); + + expect(context.messageToolPolicyHash).toBeDefined(); + expect(context.reusableCliSession).toEqual({ invalidatedReason: "system-prompt" }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it("requires explicit message targets by default for CLI subagents", async () => { + const { dir, sessionFile } = createSessionFile(); + try { + const context = await prepareCliRunContext({ + sessionId: "session-test", + sessionKey: "agent:main:subagent:child", + sessionFile, + workspaceDir: dir, + prompt: "latest ask", + provider: "test-cli", + model: "test-model", + timeoutMs: 1_000, + runId: "run-test-subagent-message-policy", + sourceReplyDeliveryMode: "message_tool_only", + config: createCliBackendConfig(), + }); + + expect(context.params.requireExplicitMessageTarget).toBe(true); + expect(context.messageToolPolicyHash).toBe( + hashCliSessionText( + JSON.stringify({ + sourceReplyDeliveryMode: "message_tool_only", + requireExplicitMessageTarget: true, + }), + ), + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + it("uses cwd for CLI system prompt workspace guidance", async () => { const { dir, sessionFile } = createSessionFile(); const taskDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-cli-task-")); @@ -1444,6 +1509,8 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { accountId: undefined, inboundEventKind: undefined, sourceReplyDeliveryMode: undefined, + requireExplicitMessageTarget: false, + senderIsOwner: undefined, }); expect(context.systemPrompt).toContain("## Memory Recall"); expect(context.systemPrompt).toContain("tools=memory_search"); @@ -1532,6 +1599,7 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { expect(context.systemPromptReport.tools.entries).toEqual([]); expect(context.promptToolNamesHash).toBeUndefined(); expect(context.preparedBackend.env).toBeUndefined(); + expect(context.mcpDeliveryCapture).toBeUndefined(); } finally { fs.rmSync(dir, { recursive: true, force: true }); } @@ -1547,10 +1615,23 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { })); const ensureMcpLoopbackServer = vi.fn(createTestMcpLoopbackServer); const createMcpLoopbackServerConfig = vi.fn(createTestMcpLoopbackServerConfig); + const resolveMcpLoopbackScopedTools = vi.fn(() => ({ + agentId: "main", + tools: [ + { + name: "message", + label: "Message", + description: "Send a message", + parameters: { type: "object", properties: {} }, + execute: vi.fn(), + }, + ], + })); setCliRunnerPrepareTestDeps({ getActiveMcpLoopbackRuntime, ensureMcpLoopbackServer, createMcpLoopbackServerConfig, + resolveMcpLoopbackScopedTools, }); cliBackendsTesting.setDepsForTest({ resolvePluginSetupCliBackend: () => undefined, @@ -1563,7 +1644,6 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { config: { command: "native-cli", args: ["--print"], - output: "text", input: "arg", sessionMode: "existing", }, @@ -1600,6 +1680,71 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { OPENCLAW_MCP_INBOUND_EVENT_KIND: "room_event", OPENCLAW_MCP_SOURCE_REPLY_DELIVERY_MODE: "message_tool_only", OPENCLAW_MCP_REQUIRE_EXPLICIT_MESSAGE_TARGET: "true", + OPENCLAW_MCP_CLI_CAPTURE_KEY: "", + }); + expect(context.mcpDeliveryCapture).toBe(true); + expect(resolveMcpLoopbackScopedTools).toHaveBeenCalledWith( + expect.objectContaining({ + requireExplicitMessageTarget: true, + }), + ); + expect(context.systemPrompt).toContain( + "include `target` and `message`; `target` is required for this turn", + ); + expect(context.systemPrompt).not.toContain( + "The target defaults to the current source channel", + ); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it("enables gateway delivery capture for Claude-style JSONL bundle MCP", async () => { + const { dir, sessionFile } = createSessionFile(); + try { + setCliRunnerPrepareTestDeps({ + getActiveMcpLoopbackRuntime: vi.fn(() => ({ + port: 31783, + ownerToken: "loopback-owner-token", + nonOwnerToken: "loopback-non-owner-token", + })), + createMcpLoopbackServerConfig: vi.fn(createTestMcpLoopbackServerConfig), + }); + cliBackendsTesting.setDepsForTest({ + resolvePluginSetupCliBackend: () => undefined, + resolveRuntimeCliBackends: () => [ + { + id: "claude-cli", + pluginId: "anthropic", + bundleMcp: true, + bundleMcpMode: "claude-config-file", + config: { + command: "claude", + args: ["--print"], + output: "jsonl", + jsonlDialect: "claude-stream-json", + input: "stdin", + sessionMode: "existing", + }, + }, + ], + }); + + const context = await prepareCliRunContext({ + sessionId: "session-test", + sessionFile, + workspaceDir: dir, + prompt: "latest ask", + provider: "claude-cli", + model: "test-model", + timeoutMs: 1_000, + runId: "run-test-claude-delivery-capture", + config: createCliBackendConfig(), + }); + + expect(context.mcpDeliveryCapture).toBe(true); + expect(context.preparedBackend.env).toMatchObject({ + OPENCLAW_MCP_CLI_CAPTURE_KEY: "", }); } finally { fs.rmSync(dir, { recursive: true, force: true }); diff --git a/src/agents/cli-runner/prepare.ts b/src/agents/cli-runner/prepare.ts index 6577ef489d7b..392c51eff7ee 100644 --- a/src/agents/cli-runner/prepare.ts +++ b/src/agents/cli-runner/prepare.ts @@ -24,6 +24,7 @@ import type { } from "../../plugins/cli-backend.types.js"; import { buildAgentHookContextChannelFields } from "../../plugins/hook-agent-context.js"; import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js"; +import { isSubagentSessionKey } from "../../routing/session-key.js"; import { annotateInterSessionPromptText } from "../../sessions/input-provenance.js"; import { resolveSkillsPromptForRun } from "../../skills/loading/workspace.js"; import { resolveEmbeddedRunSkillEntries } from "../../skills/runtime/embedded-run-entries.js"; @@ -289,6 +290,19 @@ export async function prepareCliRunContext( params.extraSystemPromptStatic !== undefined ? hashCliSessionText(params.extraSystemPromptStatic.trim() || undefined) : hashCliSessionText(extraSystemPrompt); + const requireExplicitMessageTarget = + params.requireExplicitMessageTarget ?? isSubagentSessionKey(params.sessionKey); + const messageToolPolicyHash = + params.sourceReplyDeliveryMode !== undefined || + params.requireExplicitMessageTarget !== undefined || + requireExplicitMessageTarget + ? hashCliSessionText( + JSON.stringify({ + sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, + requireExplicitMessageTarget, + }), + ) + : undefined; const modelId = (params.model ?? "default").trim() || "default"; const normalizedModel = normalizeCliModel(modelId, backendResolved.config); @@ -359,6 +373,7 @@ export async function prepareCliRunContext( } mcpLoopbackRuntime = prepareDeps.getActiveMcpLoopbackRuntime(); } + const mcpDeliveryCaptureEnabled = bundleMcpEnabled && Boolean(mcpLoopbackRuntime); const preparedBackend = await prepareCliBundleMcpConfig({ enabled: bundleMcpEnabled, mode: backendResolved.bundleMcpMode, @@ -385,8 +400,8 @@ export async function prepareCliRunContext( OPENCLAW_MCP_CURRENT_INBOUND_AUDIO: params.currentInboundAudio === true ? "true" : "", OPENCLAW_MCP_INBOUND_EVENT_KIND: params.currentInboundEventKind ?? "", OPENCLAW_MCP_SOURCE_REPLY_DELIVERY_MODE: params.sourceReplyDeliveryMode ?? "", - OPENCLAW_MCP_REQUIRE_EXPLICIT_MESSAGE_TARGET: - params.requireExplicitMessageTarget === true ? "true" : "", + OPENCLAW_MCP_REQUIRE_EXPLICIT_MESSAGE_TARGET: requireExplicitMessageTarget ? "true" : "", + OPENCLAW_MCP_CLI_CAPTURE_KEY: "", } : undefined, warn: (message) => cliBackendLog.warn(message), @@ -476,6 +491,7 @@ export async function prepareCliRunContext( accountId: params.agentAccountId, inboundEventKind: params.currentInboundEventKind, sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, + requireExplicitMessageTarget, senderIsOwner: params.senderIsOwner, }).tools : []; @@ -492,6 +508,7 @@ export async function prepareCliRunContext( authEpoch, authEpochVersion: CLI_AUTH_EPOCH_VERSION, extraSystemPromptHash, + messageToolPolicyHash, promptToolNamesHash, cwdHash, mcpConfigHash: preparedBackendFinal.mcpConfigHash, @@ -585,6 +602,7 @@ export async function prepareCliRunContext( defaultThinkLevel: params.thinkLevel, extraSystemPrompt, sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, + requireExplicitMessageTarget, silentReplyPromptMode: params.silentReplyPromptMode, runtimeChannel, runtimeChatType: params.sessionEntry?.chatType, @@ -758,6 +776,7 @@ export async function prepareCliRunContext( ...params, config: contextEngineConfig, prompt: preparedPrompt, + ...(requireExplicitMessageTarget ? { requireExplicitMessageTarget: true } : {}), }; return { @@ -781,8 +800,10 @@ export async function prepareCliRunContext( authEpoch, authEpochVersion: CLI_AUTH_EPOCH_VERSION, extraSystemPromptHash, + messageToolPolicyHash, promptToolNamesHash, cwdHash, + ...(mcpDeliveryCaptureEnabled ? { mcpDeliveryCapture: true } : {}), }; } try { @@ -821,6 +842,7 @@ export async function prepareCliRunContext( ...params, config: contextEngineConfig, prompt: preparedPrompt, + ...(requireExplicitMessageTarget ? { requireExplicitMessageTarget: true } : {}), }; return { @@ -848,8 +870,10 @@ export async function prepareCliRunContext( authEpoch, authEpochVersion: CLI_AUTH_EPOCH_VERSION, extraSystemPromptHash, + messageToolPolicyHash, promptToolNamesHash, cwdHash, + ...(mcpDeliveryCaptureEnabled ? { mcpDeliveryCapture: true } : {}), }; } catch (err) { try { diff --git a/src/agents/cli-runner/types.ts b/src/agents/cli-runner/types.ts index cf6da85cee81..573b62db85ba 100644 --- a/src/agents/cli-runner/types.ts +++ b/src/agents/cli-runner/types.ts @@ -188,6 +188,8 @@ export type PreparedCliRunContext = { authEpoch?: string; authEpochVersion: number; extraSystemPromptHash?: string; + messageToolPolicyHash?: string; promptToolNamesHash?: string; cwdHash?: string; + mcpDeliveryCapture?: true; }; diff --git a/src/agents/cli-session.test.ts b/src/agents/cli-session.test.ts index 38335bc0bac5..adca3d244ca9 100644 --- a/src/agents/cli-session.test.ts +++ b/src/agents/cli-session.test.ts @@ -27,6 +27,7 @@ describe("cli-session helpers", () => { authEpoch: "auth-epoch", authEpochVersion: 2, extraSystemPromptHash: "prompt-hash", + messageToolPolicyHash: "message-policy-hash", promptToolNamesHash: "prompt-tools-hash", cwdHash: "cwd-hash", mcpConfigHash: "mcp-hash", @@ -42,6 +43,7 @@ describe("cli-session helpers", () => { authEpoch: "auth-epoch", authEpochVersion: 2, extraSystemPromptHash: "prompt-hash", + messageToolPolicyHash: "message-policy-hash", promptToolNamesHash: "prompt-tools-hash", cwdHash: "cwd-hash", mcpConfigHash: "mcp-hash", @@ -186,6 +188,29 @@ describe("cli-session helpers", () => { ).toEqual({ invalidatedReason: "mcp" }); }); + it("invalidates reuse when message-tool prompt policy changes", () => { + const binding = { + sessionId: "cli-session-1", + authEpochVersion: 2, + messageToolPolicyHash: "message-policy-a", + }; + + expect( + resolveCliSessionReuse({ + binding, + authEpochVersion: 2, + messageToolPolicyHash: "message-policy-b", + }), + ).toEqual({ invalidatedReason: "system-prompt" }); + expect( + resolveCliSessionReuse({ + binding, + authEpochVersion: 2, + messageToolPolicyHash: "message-policy-a", + }), + ).toEqual({ sessionId: "cli-session-1" }); + }); + it("invalidates reuse when the task cwd changes", () => { const binding = { sessionId: "cli-session-1", diff --git a/src/agents/cli-session.ts b/src/agents/cli-session.ts index bd875afeefd3..f6720b73153b 100644 --- a/src/agents/cli-session.ts +++ b/src/agents/cli-session.ts @@ -38,6 +38,7 @@ export function getCliSessionBinding( authEpoch: normalizeOptionalString(fromBindings?.authEpoch), authEpochVersion: fromBindings?.authEpochVersion, extraSystemPromptHash: normalizeOptionalString(fromBindings?.extraSystemPromptHash), + messageToolPolicyHash: normalizeOptionalString(fromBindings?.messageToolPolicyHash), promptToolNamesHash: normalizeOptionalString(fromBindings?.promptToolNamesHash), cwdHash: normalizeOptionalString(fromBindings?.cwdHash), mcpConfigHash: normalizeOptionalString(fromBindings?.mcpConfigHash), @@ -100,6 +101,9 @@ export function setCliSessionBinding( ...(normalizeOptionalString(binding.extraSystemPromptHash) ? { extraSystemPromptHash: normalizeOptionalString(binding.extraSystemPromptHash) } : {}), + ...(normalizeOptionalString(binding.messageToolPolicyHash) + ? { messageToolPolicyHash: normalizeOptionalString(binding.messageToolPolicyHash) } + : {}), ...(normalizeOptionalString(binding.promptToolNamesHash) ? { promptToolNamesHash: normalizeOptionalString(binding.promptToolNamesHash) } : {}), @@ -157,6 +161,7 @@ export function resolveCliSessionReuse(params: { authEpoch?: string; authEpochVersion: number; extraSystemPromptHash?: string; + messageToolPolicyHash?: string; promptToolNamesHash?: string; cwdHash?: string; mcpConfigHash?: string; @@ -176,6 +181,7 @@ export function resolveCliSessionReuse(params: { const currentAuthProfileId = normalizeOptionalString(params.authProfileId); const currentAuthEpoch = normalizeOptionalString(params.authEpoch); const currentExtraSystemPromptHash = normalizeOptionalString(params.extraSystemPromptHash); + const currentMessageToolPolicyHash = normalizeOptionalString(params.messageToolPolicyHash); const currentPromptToolNamesHash = normalizeOptionalString(params.promptToolNamesHash); const currentCwdHash = normalizeOptionalString(params.cwdHash); const currentMcpConfigHash = normalizeOptionalString(params.mcpConfigHash); @@ -202,6 +208,10 @@ export function resolveCliSessionReuse(params: { if (storedExtraSystemPromptHash !== currentExtraSystemPromptHash) { return { invalidatedReason: "system-prompt" }; } + const storedMessageToolPolicyHash = normalizeOptionalString(binding?.messageToolPolicyHash); + if (storedMessageToolPolicyHash !== currentMessageToolPolicyHash) { + return { invalidatedReason: "system-prompt" }; + } const storedPromptToolNamesHash = normalizeOptionalString(binding?.promptToolNamesHash); if (storedPromptToolNamesHash !== currentPromptToolNamesHash) { return { invalidatedReason: "system-prompt" }; diff --git a/src/agents/command/attempt-execution.cli.test.ts b/src/agents/command/attempt-execution.cli.test.ts index 7cf6f34320dd..d17358d40c1a 100644 --- a/src/agents/command/attempt-execution.cli.test.ts +++ b/src/agents/command/attempt-execution.cli.test.ts @@ -1165,6 +1165,54 @@ describe("CLI attempt execution", () => { }); }); + it("forwards message-tool-only policy and requires explicit subagent targets", async () => { + const sessionKey = "agent:main:subagent:claude-message-policy"; + const sessionEntry: SessionEntry = { + sessionId: "openclaw-session-cli-message-policy", + updatedAt: Date.now(), + }; + const sessionStore: Record = { [sessionKey]: sessionEntry }; + await fs.writeFile(storePath, JSON.stringify(sessionStore, null, 2), "utf-8"); + runCliAgentMock.mockResolvedValueOnce(makeCliResult("sent")); + + await runAgentAttempt({ + providerOverride: "claude-cli", + originalProvider: "claude-cli", + modelOverride: "opus", + cfg: {} as OpenClawConfig, + sessionEntry, + sessionId: sessionEntry.sessionId, + sessionKey, + sessionAgentId: "main", + sessionFile: path.join(tmpDir, "session.jsonl"), + workspaceDir: tmpDir, + body: "route this", + isFallbackRetry: false, + resolvedThinkLevel: "medium", + timeoutMs: 1_000, + runId: "run-cli-message-policy", + opts: { + sourceReplyDeliveryMode: "message_tool_only", + } as Parameters[0]["opts"], + runContext: {} as Parameters[0]["runContext"], + spawnedBy: undefined, + messageChannel: "discord", + skillsSnapshot: undefined, + resolvedVerboseLevel: undefined, + agentDir: tmpDir, + onAgentEvent: vi.fn(), + authProfileProvider: "claude-cli", + sessionStore, + storePath, + sessionHasHistory: false, + }); + + expectMockArgFields(runCliAgentMock, { + sourceReplyDeliveryMode: "message_tool_only", + requireExplicitMessageTarget: true, + }); + }); + it("forwards runtime toolsAllow into CLI attempts so the CLI harness can fail closed", async () => { const sessionKey = "agent:main:direct:claude-tools-allow"; const sessionEntry: SessionEntry = { diff --git a/src/agents/command/attempt-execution.ts b/src/agents/command/attempt-execution.ts index fde394482af1..6b063091b4ff 100644 --- a/src/agents/command/attempt-execution.ts +++ b/src/agents/command/attempt-execution.ts @@ -22,6 +22,7 @@ import { readErrorName } from "../../infra/errors.js"; import { redactSensitiveText } from "../../logging/redact.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import type { PluginMetadataSnapshot } from "../../plugins/plugin-metadata-snapshot.types.js"; +import { isSubagentSessionKey } from "../../routing/session-key.js"; import { annotateInterSessionPromptText } from "../../sessions/input-provenance.js"; import { emitSessionTranscriptUpdate } from "../../sessions/transcript-events.js"; import { @@ -626,6 +627,8 @@ export function runAgentAttempt(params: { lane: params.opts.lane, extraSystemPrompt: params.opts.extraSystemPrompt, inputProvenance: params.opts.inputProvenance, + sourceReplyDeliveryMode: params.opts.sourceReplyDeliveryMode, + requireExplicitMessageTarget: isSubagentSessionKey(params.sessionKey), cliSessionId: nextCliSessionId, cliSessionBinding: nextCliSessionId === activeCliSessionBinding?.sessionId diff --git a/src/agents/embedded-agent-message-tool-source-reply.test.ts b/src/agents/embedded-agent-message-tool-source-reply.test.ts new file mode 100644 index 000000000000..a7c7806995e4 --- /dev/null +++ b/src/agents/embedded-agent-message-tool-source-reply.test.ts @@ -0,0 +1,298 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; +import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js"; +import { + isDeliveredMessageToolOnlySourceReplyResult, + isDeliveredMessagingToolResult, +} from "./embedded-agent-message-tool-source-reply.js"; +import { + isMessagingToolDeliveryAction, + isMessagingToolSendAction, +} from "./embedded-agent-messaging.js"; + +beforeEach(() => { + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "native-messaging", + source: "test", + plugin: { + ...createChannelTestPluginBase({ id: "native-messaging" }), + actions: { + describeMessageTool: () => null, + isToolDeliveryAction: ({ args }: { args: Record }) => + args.action === "editMessage" || args.action === "deleteMessage", + }, + }, + }, + ]), + ); +}); + +describe("messaging delivery action classification", () => { + it("keeps visible side effects broader than terminal reply sends", () => { + expect(isMessagingToolSendAction("message", { action: "poll" })).toBe(false); + expect(isMessagingToolDeliveryAction("message", { action: "poll" })).toBe(true); + expect(isMessagingToolDeliveryAction("message", { action: "broadcast" })).toBe(true); + expect(isMessagingToolDeliveryAction("message", { action: "thread-create" })).toBe(true); + expect(isMessagingToolDeliveryAction("message", { action: "topic-create" })).toBe(true); + expect(isMessagingToolDeliveryAction("message", { action: "createForumTopic" })).toBe(true); + expect(isMessagingToolDeliveryAction("message", { action: "channel-create" })).toBe(true); + expect(isMessagingToolDeliveryAction("message", { action: "event-create" })).toBe(true); + expect(isMessagingToolDeliveryAction("message", { action: "react" })).toBe(true); + expect(isMessagingToolDeliveryAction("message", { action: "read" })).toBe(false); + expect(isMessagingToolDeliveryAction("message", { action: "channel-list" })).toBe(false); + }); + + it("uses provider-native mutation contracts", () => { + expect(isMessagingToolDeliveryAction("native-messaging", { action: "editMessage" })).toBe(true); + expect(isMessagingToolDeliveryAction("native-messaging", { action: "deleteMessage" })).toBe( + true, + ); + expect(isMessagingToolDeliveryAction("native-messaging", { action: "readMessages" })).toBe( + false, + ); + }); +}); + +describe("isDeliveredMessagingToolResult", () => { + it("accepts confirmed delivery receipts from direct CLI text blocks", () => { + expect( + isDeliveredMessagingToolResult({ + result: [{ type: "text", text: JSON.stringify({ result: { messageId: "msg-1" } }) }], + }), + ).toBe(true); + expect(isDeliveredMessagingToolResult({ result: { status: "sent" } })).toBe(true); + }); + + it("rejects bare success markers without delivery evidence", () => { + expect(isDeliveredMessagingToolResult({ result: { ok: true, to: "spaces/AAA" } })).toBe(false); + }); + + it("accepts action-specific bare success delivery contracts", () => { + expect( + isDeliveredMessagingToolResult({ + args: { action: "poll" }, + result: { ok: true }, + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + args: { action: "sticker" }, + result: [{ type: "text", text: JSON.stringify({ ok: true }) }], + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + args: { action: "channel-create" }, + result: { ok: true }, + }), + ).toBe(true); + }); + + it("rejects successful no-op mutation results", () => { + for (const result of [ + { ok: true, removed: null }, + { ok: true, removed: 0 }, + { ok: true, removed: [] }, + { ok: true, changed: false }, + ]) { + expect( + isDeliveredMessagingToolResult({ + args: { action: "react" }, + result, + }), + ).toBe(false); + } + }); + + it("accepts sessions_send acknowledgement statuses only for sessions_send", () => { + expect( + isDeliveredMessagingToolResult({ + toolName: "sessions_send", + result: { status: "accepted" }, + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + toolName: "sessions_send", + result: { status: "ok" }, + }), + ).toBe(true); + expect(isDeliveredMessagingToolResult({ toolName: "message", result: { status: "ok" } })).toBe( + false, + ); + }); + + it("accepts post-start sessions_send timeout and error evidence", () => { + expect( + isDeliveredMessagingToolResult({ + toolName: "sessions_send", + result: { status: "timeout", sentBeforeError: true }, + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + toolName: "sessions_send", + result: { status: "error", sentBeforeError: true }, + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + toolName: "sessions_send", + result: { status: "timeout" }, + }), + ).toBe(false); + expect( + isDeliveredMessagingToolResult({ + toolName: "sessions_send", + result: { status: "error" }, + }), + ).toBe(false); + }); + + it("accepts poll delivery identifiers", () => { + expect(isDeliveredMessagingToolResult({ result: { pollId: "poll-1" } })).toBe(true); + }); + + it("accepts successful thread and topic creation receipts", () => { + expect( + isDeliveredMessagingToolResult({ + args: { action: "thread-create" }, + result: { ok: true, thread: { id: "thread-1" } }, + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + args: { action: "topic-create" }, + result: { ok: true, topicId: 42 }, + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + args: { action: "topic-create" }, + isError: true, + result: { topicId: 43, error: "post-create metadata update failed" }, + }), + ).toBe(true); + }); + + it("accepts only broadcast result entries with concrete delivery evidence", () => { + expect( + isDeliveredMessagingToolResult({ + toolName: "message", + result: { results: [{ ok: true, messageId: "message-1" }] }, + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + args: { action: "broadcast" }, + result: { + results: [ + { + channel: "telegram", + to: "chat-1", + ok: true, + payload: { ok: true, messageId: "gateway-message-1" }, + }, + ], + }, + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + args: { action: "broadcast" }, + result: { results: [{ channel: "googlechat", to: "space-1", ok: true }] }, + }), + ).toBe(false); + }); + + it("rejects successful broadcast wrappers around suppressed sends", () => { + expect( + isDeliveredMessagingToolResult({ + toolName: "message", + args: { action: "broadcast" }, + result: { results: [{ ok: true, result: { messageId: "suppressed" } }] }, + }), + ).toBe(false); + expect( + isDeliveredMessagingToolResult({ + toolName: "message", + args: { action: "broadcast" }, + result: { results: [{ ok: true, result: { deliveryStatus: "suppressed" } }] }, + }), + ).toBe(false); + }); + + it("accepts failed broadcast entries with partial-delivery evidence", () => { + expect( + isDeliveredMessagingToolResult({ + args: { action: "broadcast" }, + result: { + results: [{ channel: "telegram", to: "chat-1", ok: false, sentBeforeError: true }], + }, + }), + ).toBe(true); + }); + + it("rejects non-delivery message id sentinels", () => { + expect(isDeliveredMessagingToolResult({ result: { messageId: "skipped" } })).toBe(false); + expect(isDeliveredMessagingToolResult({ result: { messageId: "suppressed" } })).toBe(false); + }); + + it("accepts successful sends with an unknown message id", () => { + expect(isDeliveredMessagingToolResult({ result: { messageId: "unknown" } })).toBe(true); + }); + + it("rejects dry-run, suppressed, and errored results", () => { + expect( + isDeliveredMessagingToolResult({ + args: { dryRun: true }, + result: { result: { messageId: "msg-1" } }, + }), + ).toBe(false); + expect(isDeliveredMessagingToolResult({ result: { status: "suppressed" } })).toBe(false); + expect( + isDeliveredMessagingToolResult({ + isError: true, + result: { result: { messageId: "msg-1" } }, + }), + ).toBe(false); + }); + + it("accepts errored results that prove partial visible delivery", () => { + expect( + isDeliveredMessagingToolResult({ + isError: true, + result: Object.assign(new Error("second chunk failed"), { sentBeforeError: true }), + }), + ).toBe(true); + expect( + isDeliveredMessagingToolResult({ + isError: true, + result: { deliveryStatus: "partial_failed" }, + }), + ).toBe(true); + }); +}); + +describe("isDeliveredMessageToolOnlySourceReplyResult", () => { + it("accepts only confirmed implicit message sends", () => { + expect( + isDeliveredMessageToolOnlySourceReplyResult({ + sourceReplyDeliveryMode: "message_tool_only", + toolName: "message", + args: { action: "send", message: "reply" }, + result: { deliveryStatus: "sent" }, + }), + ).toBe(true); + expect( + isDeliveredMessageToolOnlySourceReplyResult({ + sourceReplyDeliveryMode: "message_tool_only", + toolName: "message", + args: { action: "send", target: "elsewhere", message: "reply" }, + result: { deliveryStatus: "sent" }, + }), + ).toBe(false); + }); +}); diff --git a/src/agents/embedded-agent-message-tool-source-reply.ts b/src/agents/embedded-agent-message-tool-source-reply.ts index fd3f9a414d33..f9024031142e 100644 --- a/src/agents/embedded-agent-message-tool-source-reply.ts +++ b/src/agents/embedded-agent-message-tool-source-reply.ts @@ -2,14 +2,20 @@ * Detects message-tool sends that delivered a visible reply to the current source. */ import type { SourceReplyDeliveryMode } from "../auto-reply/get-reply-options.types.js"; -import { isMessageToolSendActionName } from "./embedded-agent-messaging.js"; +import { + isMessageToolSendActionName, + isMessagingToolDeliveryAction, +} from "./embedded-agent-messaging.js"; import { isToolResultError } from "./embedded-agent-subscribe.tools.js"; import { normalizeToolName } from "./tool-policy.js"; const MESSAGE_TOOL_NAME = "message"; +const SESSIONS_SEND_TOOL_NAME = "sessions_send"; const EXPLICIT_MESSAGE_ROUTE_KEYS = ["channel", "target", "to", "channelId", "provider"]; const DRY_RUN_DELIVERY_STATUS = "dry_run"; +const PARTIAL_FAILED_DELIVERY_STATUS = "partial_failed"; const SENT_DELIVERY_STATUS = "sent"; +const NON_DELIVERY_MESSAGE_IDS = new Set(["skipped", "suppressed"]); const RESULT_ENVELOPE_KEYS = [ "details", "payload", @@ -18,6 +24,14 @@ const RESULT_ENVELOPE_KEYS = [ "sendResult", "toolResult", ]; +const PARTIAL_DELIVERY_ENVELOPE_KEYS = [...RESULT_ENVELOPE_KEYS, "error", "cause"]; +const SESSIONS_SEND_DELIVERY_STATUSES = new Set(["accepted", "ok"]); +const CONVERSATION_CREATE_ACTIONS = new Set([ + "thread-create", + "topic-create", + "threadcreate", + "createforumtopic", +]); function asRecord(value: unknown): Record { return value && typeof value === "object" && !Array.isArray(value) @@ -29,6 +43,10 @@ function hasStringValue(value: unknown): boolean { return typeof value === "string" && value.trim().length > 0; } +function hasConversationIdValue(value: unknown): boolean { + return hasStringValue(value) || (typeof value === "number" && Number.isFinite(value)); +} + function hasExplicitMessageRoute(args: Record): boolean { if (EXPLICIT_MESSAGE_ROUTE_KEYS.some((key) => hasStringValue(args[key]))) { return true; @@ -52,7 +70,11 @@ function parseJsonRecord(value: string): Record | undefined { } function recordHasDeliveredMessageId(record: Record): boolean { - if (hasStringValue(record.messageId)) { + const hasDeliveredId = (value: unknown) => { + const normalized = normalizeStatus(value); + return Boolean(normalized && !NON_DELIVERY_MESSAGE_IDS.has(normalized)); + }; + if (hasDeliveredId(record.messageId) || hasDeliveredId(record.pollId)) { return true; } const receipt = record.receipt; @@ -61,9 +83,191 @@ function recordHasDeliveredMessageId(record: Record): boolean { } const receiptRecord = receipt as Record; return ( - hasStringValue(receiptRecord.primaryPlatformMessageId) || + hasDeliveredId(receiptRecord.primaryPlatformMessageId) || (Array.isArray(receiptRecord.platformMessageIds) && - receiptRecord.platformMessageIds.some((value) => hasStringValue(value))) + receiptRecord.platformMessageIds.some((value) => hasDeliveredId(value))) + ); +} + +function deliveryEnvelopeHasCreatedConversationId(value: unknown, depth = 0): boolean { + if (!value || typeof value !== "object" || depth > 4) { + return false; + } + if (Array.isArray(value)) { + return value.some((item) => deliveryEnvelopeHasCreatedConversationId(item, depth + 1)); + } + + const record = value as Record; + if ( + hasConversationIdValue(record.topicId) || + hasConversationIdValue(record.threadId) || + hasConversationIdValue(record.messageThreadId) + ) { + return true; + } + const thread = record.thread; + if (thread && typeof thread === "object" && !Array.isArray(thread)) { + if (hasConversationIdValue((thread as Record).id)) { + return true; + } + } + if (typeof record.text === "string") { + const parsed = parseJsonRecord(record.text); + if (parsed && deliveryEnvelopeHasCreatedConversationId(parsed, depth + 1)) { + return true; + } + } + const content = record.content; + if ( + Array.isArray(content) && + content.some((item) => deliveryEnvelopeHasCreatedConversationId(item, depth + 1)) + ) { + return true; + } + return PARTIAL_DELIVERY_ENVELOPE_KEYS.some((key) => + deliveryEnvelopeHasCreatedConversationId(record[key], depth + 1), + ); +} + +function deliveryEnvelopeIndicatesOk(value: unknown, depth = 0): boolean { + if (!value || typeof value !== "object" || depth > 4) { + return false; + } + if (Array.isArray(value)) { + return value.some((item) => deliveryEnvelopeIndicatesOk(item, depth + 1)); + } + const record = value as Record; + if (record.ok === true) { + return true; + } + if (typeof record.text === "string") { + const parsed = parseJsonRecord(record.text); + if (parsed && deliveryEnvelopeIndicatesOk(parsed, depth + 1)) { + return true; + } + } + const content = record.content; + if ( + Array.isArray(content) && + content.some((item) => deliveryEnvelopeIndicatesOk(item, depth + 1)) + ) { + return true; + } + return RESULT_ENVELOPE_KEYS.some((key) => deliveryEnvelopeIndicatesOk(record[key], depth + 1)); +} + +function deliveryEnvelopeIndicatesNonDelivery(value: unknown, depth = 0): boolean { + if (!value || typeof value !== "object" || depth > 4) { + return false; + } + if (Array.isArray(value)) { + return value.some((item) => deliveryEnvelopeIndicatesNonDelivery(item, depth + 1)); + } + const record = value as Record; + const messageId = normalizeStatus(record.messageId); + if ( + (messageId && NON_DELIVERY_MESSAGE_IDS.has(messageId)) || + normalizeStatus(record.deliveryStatus) === "suppressed" || + normalizeStatus(record.status) === "suppressed" + ) { + return true; + } + if (typeof record.text === "string") { + const parsed = parseJsonRecord(record.text); + if (parsed && deliveryEnvelopeIndicatesNonDelivery(parsed, depth + 1)) { + return true; + } + } + const content = record.content; + if ( + Array.isArray(content) && + content.some((item) => deliveryEnvelopeIndicatesNonDelivery(item, depth + 1)) + ) { + return true; + } + return RESULT_ENVELOPE_KEYS.some((key) => + deliveryEnvelopeIndicatesNonDelivery(record[key], depth + 1), + ); +} + +function deliveryEnvelopeIndicatesNoOp(value: unknown, depth = 0): boolean { + if (!value || typeof value !== "object" || depth > 4) { + return false; + } + if (Array.isArray(value)) { + return value.some((item) => deliveryEnvelopeIndicatesNoOp(item, depth + 1)); + } + const record = value as Record; + const removed = record.removed; + if ( + removed === null || + removed === false || + removed === 0 || + (Array.isArray(removed) && removed.length === 0) || + record.applied === false || + record.changed === false || + record.created === false || + record.deleted === false || + record.sent === false || + record.updated === false + ) { + return true; + } + const status = normalizeStatus(record.status); + if (status === "noop" || status === "no_op" || status === "not_found") { + return true; + } + if (typeof record.text === "string") { + const parsed = parseJsonRecord(record.text); + if (parsed && deliveryEnvelopeIndicatesNoOp(parsed, depth + 1)) { + return true; + } + } + const content = record.content; + if ( + Array.isArray(content) && + content.some((item) => deliveryEnvelopeIndicatesNoOp(item, depth + 1)) + ) { + return true; + } + return RESULT_ENVELOPE_KEYS.some((key) => deliveryEnvelopeIndicatesNoOp(record[key], depth + 1)); +} + +function deliveryEnvelopeIndicatesSuccessfulBroadcast(value: unknown, depth = 0): boolean { + if (!value || typeof value !== "object" || depth > 4) { + return false; + } + if (Array.isArray(value)) { + return value.some( + (item) => + item !== null && + typeof item === "object" && + !Array.isArray(item) && + (item as Record).ok === true && + !deliveryEnvelopeIndicatesNonDelivery(item) && + !deliveryEnvelopeIndicatesNoOp(item) && + deliveryEnvelopeIndicatesDelivered(item, depth + 1), + ); + } + const record = value as Record; + if (deliveryEnvelopeIndicatesSuccessfulBroadcast(record.results, depth + 1)) { + return true; + } + if (typeof record.text === "string") { + const parsed = parseJsonRecord(record.text); + if (parsed && deliveryEnvelopeIndicatesSuccessfulBroadcast(parsed, depth + 1)) { + return true; + } + } + const content = record.content; + if ( + Array.isArray(content) && + content.some((item) => deliveryEnvelopeIndicatesSuccessfulBroadcast(item, depth + 1)) + ) { + return true; + } + return RESULT_ENVELOPE_KEYS.some((key) => + deliveryEnvelopeIndicatesSuccessfulBroadcast(record[key], depth + 1), ); } @@ -78,10 +282,17 @@ function deliveryEnvelopeIndicatesDryRun(value: unknown, depth = 0): boolean { const record = value as Record; if ( record.dryRun === true || - normalizeStatus(record.deliveryStatus) === DRY_RUN_DELIVERY_STATUS + normalizeStatus(record.deliveryStatus) === DRY_RUN_DELIVERY_STATUS || + normalizeStatus(record.status) === DRY_RUN_DELIVERY_STATUS ) { return true; } + if (typeof record.text === "string") { + const parsed = parseJsonRecord(record.text); + if (parsed && deliveryEnvelopeIndicatesDryRun(parsed, depth + 1)) { + return true; + } + } const content = record.content; if (Array.isArray(content)) { @@ -117,10 +328,17 @@ function deliveryEnvelopeIndicatesDelivered(value: unknown, depth = 0): boolean const record = value as Record; if ( normalizeStatus(record.deliveryStatus) === SENT_DELIVERY_STATUS || + normalizeStatus(record.status) === SENT_DELIVERY_STATUS || recordHasDeliveredMessageId(record) ) { return true; } + if (typeof record.text === "string") { + const parsed = parseJsonRecord(record.text); + if (parsed && deliveryEnvelopeIndicatesDelivered(parsed, depth + 1)) { + return true; + } + } const content = record.content; if (Array.isArray(content)) { @@ -145,6 +363,129 @@ function deliveryEnvelopeIndicatesDelivered(value: unknown, depth = 0): boolean ); } +function deliveryEnvelopeIndicatesSessionsSendAccepted(value: unknown, depth = 0): boolean { + if (!value || typeof value !== "object" || depth > 4) { + return false; + } + if (Array.isArray(value)) { + return value.some((item) => deliveryEnvelopeIndicatesSessionsSendAccepted(item, depth + 1)); + } + const record = value as Record; + if ( + SESSIONS_SEND_DELIVERY_STATUSES.has(normalizeStatus(record.deliveryStatus) ?? "") || + SESSIONS_SEND_DELIVERY_STATUSES.has(normalizeStatus(record.status) ?? "") + ) { + return true; + } + if (typeof record.text === "string") { + const parsed = parseJsonRecord(record.text); + if (parsed && deliveryEnvelopeIndicatesSessionsSendAccepted(parsed, depth + 1)) { + return true; + } + } + const content = record.content; + if ( + Array.isArray(content) && + content.some((item) => deliveryEnvelopeIndicatesSessionsSendAccepted(item, depth + 1)) + ) { + return true; + } + return RESULT_ENVELOPE_KEYS.some((key) => + deliveryEnvelopeIndicatesSessionsSendAccepted(record[key], depth + 1), + ); +} + +function deliveryEnvelopeIndicatesPartialDelivery(value: unknown, depth = 0): boolean { + if (!value || typeof value !== "object" || depth > 4) { + return false; + } + if (Array.isArray(value)) { + return value.some((item) => deliveryEnvelopeIndicatesPartialDelivery(item, depth + 1)); + } + + const record = value as Record; + if ( + record.sentBeforeError === true || + record.visibleReplySent === true || + normalizeStatus(record.deliveryStatus) === PARTIAL_FAILED_DELIVERY_STATUS || + normalizeStatus(record.status) === PARTIAL_FAILED_DELIVERY_STATUS + ) { + return true; + } + return PARTIAL_DELIVERY_ENVELOPE_KEYS.some((key) => + deliveryEnvelopeIndicatesPartialDelivery(record[key], depth + 1), + ); +} + +/** Return true only when a messaging tool result proves a real visible delivery. */ +export function isDeliveredMessagingToolResult(params: { + toolName?: string; + args?: unknown; + result?: unknown; + hookResult?: unknown; + isError?: boolean; +}): boolean { + const args = asRecord(params.args); + const action = normalizeStatus(args.action); + if ( + args.dryRun === true || + deliveryEnvelopeIndicatesDryRun(params.result) || + deliveryEnvelopeIndicatesDryRun(params.hookResult) + ) { + return false; + } + if ( + deliveryEnvelopeIndicatesPartialDelivery(params.result) || + deliveryEnvelopeIndicatesPartialDelivery(params.hookResult) + ) { + return true; + } + if ( + action && + CONVERSATION_CREATE_ACTIONS.has(action) && + (deliveryEnvelopeHasCreatedConversationId(params.result) || + deliveryEnvelopeHasCreatedConversationId(params.hookResult)) + ) { + return true; + } + if ( + action === "broadcast" && + (deliveryEnvelopeIndicatesSuccessfulBroadcast(params.result) || + deliveryEnvelopeIndicatesSuccessfulBroadcast(params.hookResult)) + ) { + return true; + } + if (params.isError || isToolResultError(params.result) || isToolResultError(params.hookResult)) { + return false; + } + const normalizedToolName = normalizeToolName(params.toolName ?? MESSAGE_TOOL_NAME); + const mutationHasBareOk = + isMessagingToolDeliveryAction(normalizedToolName, args) && + action !== "broadcast" && + (deliveryEnvelopeIndicatesOk(params.result) || deliveryEnvelopeIndicatesOk(params.hookResult)); + if ( + mutationHasBareOk && + !deliveryEnvelopeIndicatesNonDelivery(params.result) && + !deliveryEnvelopeIndicatesNonDelivery(params.hookResult) && + !deliveryEnvelopeIndicatesNoOp(params.result) && + !deliveryEnvelopeIndicatesNoOp(params.hookResult) + ) { + return true; + } + if (normalizedToolName === SESSIONS_SEND_TOOL_NAME) { + return ( + deliveryEnvelopeIndicatesSessionsSendAccepted(params.result) || + deliveryEnvelopeIndicatesSessionsSendAccepted(params.hookResult) || + deliveryEnvelopeIndicatesDelivered(params.result) || + deliveryEnvelopeIndicatesDelivered(params.hookResult) + ); + } + return ( + deliveryEnvelopeIndicatesDelivered(params.result) || + deliveryEnvelopeIndicatesDelivered(params.hookResult) + ); +} + /** * Only implicit-route, non-dry-run, delivered `message.send` calls qualify. * Explicit routes and other messaging tools are outbound side effects, not source replies. @@ -167,18 +508,5 @@ export function isDeliveredMessageToolOnlySourceReplyResult(params: { if (!isMessageToolSendActionName(args.action) || hasExplicitMessageRoute(args)) { return false; } - if (params.isError || isToolResultError(params.result) || isToolResultError(params.hookResult)) { - return false; - } - if ( - args.dryRun === true || - deliveryEnvelopeIndicatesDryRun(params.result) || - deliveryEnvelopeIndicatesDryRun(params.hookResult) - ) { - return false; - } - return ( - deliveryEnvelopeIndicatesDelivered(params.result) || - deliveryEnvelopeIndicatesDelivered(params.hookResult) - ); + return isDeliveredMessagingToolResult(params); } diff --git a/src/agents/embedded-agent-messaging.ts b/src/agents/embedded-agent-messaging.ts index 0d98a50f6750..4145240b3150 100644 --- a/src/agents/embedded-agent-messaging.ts +++ b/src/agents/embedded-agent-messaging.ts @@ -3,6 +3,7 @@ */ import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { getChannelPlugin, normalizeChannelId } from "../channels/plugins/index.js"; +import { CHANNEL_MESSAGE_ACTION_NAMES } from "../channels/plugins/types.public.js"; const CORE_MESSAGING_TOOLS = new Set(["sessions_send", "message"]); const MESSAGE_TOOL_SEND_ACTIONS = new Set([ @@ -12,6 +13,27 @@ const MESSAGE_TOOL_SEND_ACTIONS = new Set([ "sendAttachment", "upload-file", ]); +const MESSAGE_TOOL_READ_ONLY_ACTIONS = new Set([ + "read", + "reactions", + "list-pins", + "permissions", + "thread-list", + "search", + "sticker-search", + "member-info", + "role-info", + "emoji-list", + "channel-info", + "channel-list", + "voice-status", + "event-list", + "download-file", +]); +const MESSAGE_TOOL_MUTATION_ACTIONS = new Set( + CHANNEL_MESSAGE_ACTION_NAMES.filter((action) => !MESSAGE_TOOL_READ_ONLY_ACTIONS.has(action)), +); +const MESSAGE_TOOL_MUTATION_ALIASES = new Set(["threadCreate", "createForumTopic"]); /** Return true when a message action sends or uploads user-visible content. */ export function isMessageToolSendActionName(action: unknown): boolean { @@ -42,12 +64,23 @@ export function isMessagingToolSendAction( return isMessageToolSendActionName(action); } const providerId = normalizeChannelId(toolName); - if (!providerId) { - return false; - } - const plugin = getChannelPlugin(providerId); - if (!plugin?.actions?.extractToolSend) { - return false; - } - return Boolean(plugin.actions.extractToolSend({ args })?.to); + return Boolean( + providerId && getChannelPlugin(providerId)?.actions?.extractToolSend?.({ args })?.to, + ); +} + +/** Return true when a messaging invocation can create visible outbound delivery. */ +export function isMessagingToolDeliveryAction( + toolName: string, + args: Record, +): boolean { + if (toolName === "message") { + const action = normalizeOptionalString(args.action) ?? ""; + return MESSAGE_TOOL_MUTATION_ACTIONS.has(action) || MESSAGE_TOOL_MUTATION_ALIASES.has(action); + } + const providerId = normalizeChannelId(toolName); + if (providerId && getChannelPlugin(providerId)?.actions?.isToolDeliveryAction?.({ args })) { + return true; + } + return isMessagingToolSendAction(toolName, args); } diff --git a/src/agents/embedded-agent-subscribe.handlers.tools.ts b/src/agents/embedded-agent-subscribe.handlers.tools.ts index 75e20766638c..2582e42d8803 100644 --- a/src/agents/embedded-agent-subscribe.handlers.tools.ts +++ b/src/agents/embedded-agent-subscribe.handlers.tools.ts @@ -30,7 +30,6 @@ import { emitAgentPatchSummaryEvent, } from "../infra/agent-events.js"; import type { ExecApprovalDecision } from "../infra/exec-approvals.js"; -import { normalizeInteractiveReply, normalizeMessagePresentation } from "../interactive/payload.js"; import type { PluginHookAfterToolCallEvent } from "../plugins/types.js"; import { createLazyImportLoader } from "../shared/lazy-promise.js"; import { truncateUtf16Safe } from "../utils.js"; @@ -47,7 +46,6 @@ import { sanitizeForConsole } from "./console-sanitize.js"; import { normalizeTextForComparison } from "./embedded-agent-helpers.js"; import { isDeliveredMessageToolOnlySourceReplyResult } from "./embedded-agent-message-tool-source-reply.js"; import { isMessagingTool, isMessagingToolSendAction } from "./embedded-agent-messaging.js"; -import type { MessagingToolSourceReplyPayload } from "./embedded-agent-messaging.types.js"; import { mergeEmbeddedRunReplayState } from "./embedded-agent-runner/replay-state.js"; import type { ToolCallSummary, @@ -55,6 +53,9 @@ import type { } from "./embedded-agent-subscribe.handlers.types.js"; import { isPromiseLike } from "./embedded-agent-subscribe.promise.js"; import { + collectMessagingMediaUrlsFromRecord, + collectMessagingMediaUrlsFromToolResult, + extractMessagingToolSourceReplyPayload, extractToolResultMediaArtifact, extractToolErrorCode, extractMessagingToolSend, @@ -508,56 +509,6 @@ function extendExecMeta(toolName: string, args: unknown, meta?: string): string return meta ? `${meta} · ${suffix}` : suffix; } -function pushUniqueMediaUrl(urls: string[], seen: Set, value: unknown): void { - if (typeof value !== "string") { - return; - } - const normalized = value.trim(); - if (!normalized || seen.has(normalized)) { - return; - } - seen.add(normalized); - urls.push(normalized); -} - -function collectMessagingMediaUrlsFromRecord(record: Record): string[] { - const urls: string[] = []; - const seen = new Set(); - const pushAttachment = (value: unknown) => { - if (!value || typeof value !== "object" || Array.isArray(value)) { - return; - } - const attachment = value as Record; - pushUniqueMediaUrl(urls, seen, attachment.media); - pushUniqueMediaUrl(urls, seen, attachment.mediaUrl); - pushUniqueMediaUrl(urls, seen, attachment.path); - pushUniqueMediaUrl(urls, seen, attachment.filePath); - pushUniqueMediaUrl(urls, seen, attachment.fileUrl); - pushUniqueMediaUrl(urls, seen, attachment.url); - }; - - pushUniqueMediaUrl(urls, seen, record.media); - pushUniqueMediaUrl(urls, seen, record.mediaUrl); - pushUniqueMediaUrl(urls, seen, record.path); - pushUniqueMediaUrl(urls, seen, record.filePath); - pushUniqueMediaUrl(urls, seen, record.fileUrl); - - const mediaUrls = record.mediaUrls; - if (Array.isArray(mediaUrls)) { - for (const mediaUrl of mediaUrls) { - pushUniqueMediaUrl(urls, seen, mediaUrl); - } - } - const attachments = record.attachments; - if (Array.isArray(attachments)) { - for (const attachment of attachments) { - pushAttachment(attachment); - } - } - - return urls; -} - function readMessagingText(record: Record): string | undefined { for (const key of ["content", "message", "text", "body"]) { const value = readStringValue(record[key]); @@ -568,115 +519,6 @@ function readMessagingText(record: Record): string | undefined return undefined; } -function collectMessagingMediaUrlsFromToolResult(result: unknown): string[] { - const urls: string[] = []; - const seen = new Set(); - const appendFromRecord = (value: unknown) => { - if (!value || typeof value !== "object") { - return; - } - const extracted = collectMessagingMediaUrlsFromRecord(value as Record); - for (const url of extracted) { - if (seen.has(url)) { - continue; - } - seen.add(url); - urls.push(url); - } - }; - - appendFromRecord(result); - if (result && typeof result === "object") { - appendFromRecord((result as Record).details); - } - - const outputText = extractToolResultText(result); - if (outputText) { - try { - appendFromRecord(JSON.parse(outputText)); - } catch { - // Ignore non-JSON tool output. - } - } - - return urls; -} - -function readStringField(record: Record, key: string): string | undefined { - const value = record[key]; - return typeof value === "string" && value.trim() ? value : undefined; -} - -function readStringArrayField(record: Record, key: string): string[] | undefined { - const value = record[key]; - if (!Array.isArray(value)) { - return undefined; - } - const strings = value.filter( - (item): item is string => typeof item === "string" && item.trim().length > 0, - ); - return strings.length ? strings : undefined; -} - -function copyRecordField( - record: Record, - key: string, -): Record | undefined { - const value = record[key]; - return readRecordField(value) ? { ...(value as Record) } : undefined; -} - -function extractMessagingToolSourceReplyPayload( - result: unknown, -): MessagingToolSourceReplyPayload | undefined { - const details = readToolResultDetailsRecord(result); - if (!details || details.sourceReplySink !== "internal-ui") { - return undefined; - } - const status = normalizeOptionalLowercaseString(details.deliveryStatus); - if (status && status !== "sent") { - return undefined; - } - const sourceReply = readRecordField(details.sourceReply) ?? details; - const payload: MessagingToolSourceReplyPayload = {}; - const text = readStringField(sourceReply, "text") ?? readStringField(details, "message"); - if (text) { - payload.text = text; - } - const mediaUrl = readStringField(sourceReply, "mediaUrl") ?? readStringField(details, "mediaUrl"); - if (mediaUrl) { - payload.mediaUrl = mediaUrl; - } - const mediaUrls = - readStringArrayField(sourceReply, "mediaUrls") ?? readStringArrayField(details, "mediaUrls"); - if (mediaUrls) { - payload.mediaUrls = mediaUrls; - } - const audioAsVoice = - sourceReply.audioAsVoice === true || details.audioAsVoice === true ? true : undefined; - if (audioAsVoice) { - payload.audioAsVoice = true; - } - const presentation = normalizeMessagePresentation(sourceReply.presentation); - if (presentation) { - payload.presentation = presentation; - } - const interactive = normalizeInteractiveReply(sourceReply.interactive); - if (interactive) { - payload.interactive = interactive; - } - const channelData = copyRecordField(sourceReply, "channelData"); - if (channelData) { - payload.channelData = channelData; - } - const idempotencyKey = - readStringField(sourceReply, "idempotencyKey") ?? readStringField(details, "idempotencyKey"); - if (idempotencyKey) { - payload.idempotencyKey = idempotencyKey; - } - return Object.keys(payload).length > 0 ? payload : undefined; -} - function queuePendingToolMedia( ctx: ToolHandlerContext, mediaReply: { mediaUrls: string[]; audioAsVoice?: boolean; trustedLocalMedia?: boolean }, diff --git a/src/agents/embedded-agent-subscribe.tools.ts b/src/agents/embedded-agent-subscribe.tools.ts index a81475928736..564354375432 100644 --- a/src/agents/embedded-agent-subscribe.tools.ts +++ b/src/agents/embedded-agent-subscribe.tools.ts @@ -12,11 +12,15 @@ import { uniqueStrings } from "@openclaw/normalization-core/string-normalization import { getChannelPlugin, normalizeChannelId } from "../channels/plugins/index.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { normalizeTargetForProvider } from "../infra/outbound/target-normalization.js"; +import { normalizeInteractiveReply, normalizeMessagePresentation } from "../interactive/payload.js"; import { redactSensitiveFieldValue, redactToolPayloadText } from "../logging/redact.js"; import { truncateUtf16Safe } from "../utils.js"; import { collectTextContentBlocks } from "./content-blocks.js"; import { isMessageToolSendActionName } from "./embedded-agent-messaging.js"; -import type { MessagingToolSend } from "./embedded-agent-messaging.types.js"; +import type { + MessagingToolSend, + MessagingToolSourceReplyPayload, +} from "./embedded-agent-messaging.types.js"; import { normalizeToolName } from "./tool-policy.js"; import { readToolResultDetails, readToolResultStatus } from "./tool-result-error.js"; @@ -279,6 +283,148 @@ export function extractToolResultText(result: unknown): string | undefined { return texts.join("\n"); } +function pushUniqueMessagingMediaUrl(urls: string[], seen: Set, value: unknown): void { + if (typeof value !== "string") { + return; + } + const normalized = value.trim(); + if (!normalized || seen.has(normalized)) { + return; + } + seen.add(normalized); + urls.push(normalized); +} + +/** Collects messaging attachment references from tool-call arguments or result records. */ +export function collectMessagingMediaUrlsFromRecord(record: Record): string[] { + const urls: string[] = []; + const seen = new Set(); + const pushAttachment = (value: unknown) => { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return; + } + const attachment = value as Record; + for (const candidate of [ + attachment.media, + attachment.mediaUrl, + attachment.path, + attachment.filePath, + attachment.fileUrl, + attachment.url, + ]) { + pushUniqueMessagingMediaUrl(urls, seen, candidate); + } + }; + + for (const candidate of [ + record.media, + record.mediaUrl, + record.path, + record.filePath, + record.fileUrl, + ]) { + pushUniqueMessagingMediaUrl(urls, seen, candidate); + } + if (Array.isArray(record.mediaUrls)) { + for (const mediaUrl of record.mediaUrls) { + pushUniqueMessagingMediaUrl(urls, seen, mediaUrl); + } + } + if (Array.isArray(record.attachments)) { + for (const attachment of record.attachments) { + pushAttachment(attachment); + } + } + return urls; +} + +/** Collects messaging attachment references from a completed tool result. */ +export function collectMessagingMediaUrlsFromToolResult(result: unknown): string[] { + const urls: string[] = []; + const seen = new Set(); + const appendFromRecord = (value: unknown) => { + if (!value || typeof value !== "object") { + return; + } + for (const url of collectMessagingMediaUrlsFromRecord(value as Record)) { + if (!seen.has(url)) { + seen.add(url); + urls.push(url); + } + } + }; + + appendFromRecord(result); + if (result && typeof result === "object") { + appendFromRecord((result as Record).details); + } + const outputText = extractToolResultText(result); + if (outputText) { + try { + appendFromRecord(JSON.parse(outputText)); + } catch { + // Ignore non-JSON tool output. + } + } + return urls; +} + +/** Extract an internal source-reply payload from a completed message tool result. */ +export function extractMessagingToolSourceReplyPayload( + result: unknown, +): MessagingToolSourceReplyPayload | undefined { + const details = readToolResultDetails(result); + if (!details || details.sourceReplySink !== "internal-ui") { + return undefined; + } + const status = normalizeOptionalLowercaseString(details.deliveryStatus); + if (status && status !== "sent") { + return undefined; + } + const sourceReply = readRecord(details.sourceReply) ?? details; + const payload: MessagingToolSourceReplyPayload = {}; + const text = readStringValue(sourceReply.text) ?? readStringValue(details.message); + if (text) { + payload.text = text; + } + const mediaUrl = readStringValue(sourceReply.mediaUrl) ?? readStringValue(details.mediaUrl); + if (mediaUrl) { + payload.mediaUrl = mediaUrl; + } + const rawMediaUrls = Array.isArray(sourceReply.mediaUrls) + ? sourceReply.mediaUrls + : Array.isArray(details.mediaUrls) + ? details.mediaUrls + : []; + const mediaUrls = uniqueStrings( + rawMediaUrls.filter((value): value is string => typeof value === "string"), + ); + if (mediaUrls.length > 0) { + payload.mediaUrls = mediaUrls; + } + if (sourceReply.audioAsVoice === true || details.audioAsVoice === true) { + payload.audioAsVoice = true; + } + const presentation = normalizeMessagePresentation(sourceReply.presentation); + if (presentation) { + payload.presentation = presentation; + } + const interactive = normalizeInteractiveReply(sourceReply.interactive); + if (interactive) { + payload.interactive = interactive; + } + const channelData = readRecord(sourceReply.channelData); + if (channelData) { + payload.channelData = { ...channelData }; + } + const idempotencyKey = + readStringValue(sourceReply.idempotencyKey) ?? readStringValue(details.idempotencyKey); + if (idempotencyKey) { + payload.idempotencyKey = idempotencyKey; + } + return Object.keys(payload).length > 0 ? payload : undefined; +} + // Core tool names that are allowed to emit trusted local media artifacts. // Plugin tools must be explicitly passed as trusted run-local names by the caller. const TRUSTED_TOOL_RESULT_MEDIA = new Set([ diff --git a/src/agents/openclaw-tools.sessions.test.ts b/src/agents/openclaw-tools.sessions.test.ts index 0a90db43d64a..fa8911db8a50 100644 --- a/src/agents/openclaw-tools.sessions.test.ts +++ b/src/agents/openclaw-tools.sessions.test.ts @@ -198,6 +198,7 @@ type SessionsSendDetails = { runId?: string; reply?: string; error?: string; + sentBeforeError?: boolean; sessionKey?: string; delivery?: { status?: string; @@ -1223,6 +1224,7 @@ describe("sessions tools", () => { expect(details.status).toBe("timeout"); expect(details.error).toBe("429 RESOURCE_EXHAUSTED"); expect(details.runId).toBe("run-pending-model-error"); + expect(details.sentBeforeError).toBe(true); expect(details.delivery?.status).toBe("pending"); expect(calls.filter((call) => call.method === "agent")).toHaveLength(1); await vi.waitFor(() => @@ -1949,6 +1951,7 @@ describe("sessions tools", () => { const details = sessionsSendDetails(result.details); expect(details.status).toBe("timeout"); expect(details.error).toBe("agent run timed out"); + expect(details.sentBeforeError).toBe(true); expect(details.sessionKey).toBe(targetKey); await new Promise((resolve) => { setImmediate(resolve); @@ -1956,6 +1959,42 @@ describe("sessions tools", () => { expect(countMatching(calls, (call) => call.method === "agent")).toBe(1); }); + it("sessions_send preserves delivery evidence for post-start agent errors", async () => { + const targetKey = "agent:director1:main"; + callGatewayMock.mockImplementation(async (opts: unknown) => { + const request = opts as { method?: string }; + if (request.method === "agent") { + return { runId: "run-error", status: "accepted", acceptedAt: 2000 }; + } + if (request.method === "agent.wait") { + return { runId: "run-error", status: "error", error: "agent failed" }; + } + if (request.method === "chat.history") { + return { messages: [] }; + } + return {}; + }); + + const tool = createOpenClawTools({ + agentSessionKey: "agent:main:main", + agentChannel: "discord", + }).find((candidate) => candidate.name === "sessions_send"); + if (!tool) { + throw new Error("missing sessions_send tool"); + } + + const result = await tool.execute("call-error", { + sessionKey: targetKey, + message: "ping", + timeoutSeconds: 1, + }); + const details = sessionsSendDetails(result.details); + expect(details.status).toBe("error"); + expect(details.error).toBe("agent failed"); + expect(details.sentBeforeError).toBe(true); + expect(details.sessionKey).toBe(targetKey); + }); + it("sessions_send skips duplicate A2A delivery for waited parent-owned native subagents", async () => { const calls: Array<{ method?: string; params?: unknown }> = []; const requesterKey = "agent:main:discord:direct:parent"; diff --git a/src/agents/system-prompt.test.ts b/src/agents/system-prompt.test.ts index 0660f36bb671..4d1b27511e7c 100644 --- a/src/agents/system-prompt.test.ts +++ b/src/agents/system-prompt.test.ts @@ -1081,6 +1081,22 @@ describe("buildAgentSystemPrompt", () => { }, ); + it("requires an explicit target for message-tool-only turns when requested", () => { + const prompt = buildAgentSystemPrompt({ + workspaceDir: "/tmp/openclaw", + toolNames: ["message"], + sourceReplyDeliveryMode: "message_tool_only", + requireExplicitMessageTarget: true, + runtimeInfo: { + channel: "telegram", + chatType: "group", + }, + }); + + expect(prompt).toContain("include `target` and `message`; `target` is required for this turn"); + expect(prompt).not.toContain("The target defaults to the current source channel"); + }); + it("tells automatic source delivery to expose generated media as MEDIA directives", () => { const prompt = buildAgentSystemPrompt({ workspaceDir: "/tmp/openclaw", diff --git a/src/agents/system-prompt.ts b/src/agents/system-prompt.ts index 40f1a6e2773a..e793375c1e87 100644 --- a/src/agents/system-prompt.ts +++ b/src/agents/system-prompt.ts @@ -492,6 +492,7 @@ function buildMessagingSection(params: { messageChannelOptions?: string; messageToolHints?: string[]; sourceReplyDeliveryMode?: SourceReplyDeliveryMode; + requireExplicitMessageTarget?: boolean; silentReplyPromptMode?: SilentReplyPromptMode; }) { if (params.isMinimal) { @@ -539,7 +540,9 @@ function buildMessagingSection(params: { ? "- Discord group/thread etiquette: a mention plus message-tool-only delivery does not require visible output. For stale threads, jokes, lightweight acknowledgements, or low-value chatter, prefer a reaction or no channel message; post only when you have concrete value to add." : "", messageToolOnly - ? "- For `action=send`, include `message`. The target defaults to the current source channel; include `target` only when sending somewhere else." + ? params.requireExplicitMessageTarget + ? "- For `action=send`, include `target` and `message`; `target` is required for this turn." + : "- For `action=send`, include `message`. The target defaults to the current source channel; include `target` only when sending somewhere else." : "- For `action=send`, include `target` and `message`.", params.messageChannelOptions ? `- No current/default source channel: include \`channel\` for proactive sends; valid ids: ${params.messageChannelOptions}.` @@ -704,6 +707,7 @@ export function buildAgentSystemPrompt(params: { /** Controls the generic silent-reply section. Channel-aware prompts can set "none". */ silentReplyPromptMode?: SilentReplyPromptMode; sourceReplyDeliveryMode?: SourceReplyDeliveryMode; + requireExplicitMessageTarget?: boolean; /** Prompt-only strength for delegating non-trivial work through sub-agents. Defaults to "suggest". */ subagentDelegationMode?: SubagentDelegationMode; /** Whether ACP-specific routing guidance should be included. Defaults to true. */ @@ -1299,6 +1303,7 @@ export function buildAgentSystemPrompt(params: { messageChannelOptions, messageToolHints: params.messageToolHints, sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, + requireExplicitMessageTarget: params.requireExplicitMessageTarget, silentReplyPromptMode, }), ...buildVoiceSection({ isMinimal, ttsHint: params.ttsHint }), diff --git a/src/agents/tool-loop-detection.test.ts b/src/agents/tool-loop-detection.test.ts index 8f39dafd6b7d..d9b6cdb93c33 100644 --- a/src/agents/tool-loop-detection.test.ts +++ b/src/agents/tool-loop-detection.test.ts @@ -1121,6 +1121,18 @@ describe("tool-loop-detection", () => { expect(hashes?.[0]).not.toBe(hashes?.[1]); }); + it("keeps full result hashing for administrative message mutations", () => { + const state = createState(); + const params = { action: "channel-create", name: "support" }; + recordSend(state, "message", params, { ok: true, messageId: "m_0", timestamp: 1000 }, 0); + recordSend(state, "message", params, { ok: true, messageId: "m_1", timestamp: 2000 }, 1); + const hashes = state.toolCallHistory + ?.filter((call) => call.toolName === "message") + .map((call) => call.resultHash); + expect(hashes?.[0]).toBeTypeOf("string"); + expect(hashes?.[0]).not.toBe(hashes?.[1]); + }); + it("preserves stable nested route ids so distinct routes stay distinguishable", () => { const state = createState(); const params = { action: "send", target: "feishu:oc_chat", text: "ping" }; diff --git a/src/agents/tools/sessions-send-tool.ts b/src/agents/tools/sessions-send-tool.ts index d351f624e954..b4059c077443 100644 --- a/src/agents/tools/sessions-send-tool.ts +++ b/src/agents/tools/sessions-send-tool.ts @@ -718,6 +718,7 @@ export function createSessionsSendTool(opts?: { runId, status: "timeout", error: result.error, + sentBeforeError: true, sessionKey: displayKey, delivery, }); @@ -735,6 +736,7 @@ export function createSessionsSendTool(opts?: { runId, status: "timeout", error: result.error, + sentBeforeError: true, sessionKey: displayKey, }); } @@ -743,6 +745,7 @@ export function createSessionsSendTool(opts?: { runId, status: "error", error: result.error ?? "agent error", + sentBeforeError: true, sessionKey: displayKey, }); } diff --git a/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts b/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts index 5b1d9d6c6ade..3c85fc30aef8 100644 --- a/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts +++ b/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts @@ -1703,6 +1703,54 @@ describe("runReplyAgent typing (heartbeat)", () => { } }); + it("does not report silent fallback failure after a did-send-only side effect", async () => { + state.runEmbeddedAgentMock.mockResolvedValueOnce({ + payloads: [], + didSendViaMessagingTool: true, + meta: {}, + }); + const fallbackSpy = vi + .spyOn(modelFallbackModule, "runWithModelFallback") + .mockImplementationOnce( + async ({ run }: { run: (provider: string, model: string) => Promise }) => ({ + result: await run("openai", "gpt-5.5"), + provider: "openai", + model: "gpt-5.5", + attempts: [ + { + provider: "lmstudio", + model: "gemma-4-e4b-it", + error: "Connection error.", + reason: "timeout", + }, + ], + }), + ); + + try { + const { run } = createMinimalRun({ + runOverrides: { + provider: "lmstudio", + model: "gemma-4-e4b-it", + }, + sessionCtx: { + Provider: "discord", + OriginatingChannel: "discord", + MessageSid: "1503645939964055592", + }, + }); + + const res = await run(); + const payload = Array.isArray(res) ? res[0] : res; + + expect(payload?.isError).not.toBe(true); + expect(payload?.text).toContain("Model Fallback:"); + expect(payload?.text).not.toContain("no visible reply"); + } finally { + fallbackSpy.mockRestore(); + } + }); + it("does not treat whitespace-only messaging evidence as fallback delivery", async () => { state.runEmbeddedAgentMock.mockResolvedValueOnce({ payloads: [{ text: "NO_REPLY" }], diff --git a/src/auto-reply/reply/agent-runner.ts b/src/auto-reply/reply/agent-runner.ts index e243ad75ed6d..9429e0b53e65 100644 --- a/src/auto-reply/reply/agent-runner.ts +++ b/src/auto-reply/reply/agent-runner.ts @@ -250,10 +250,12 @@ function hasSuccessfulSideEffectDelivery(params: { messagingToolSentTexts?: string[]; messagingToolSentMediaUrls?: string[]; messagingToolSentTargets?: unknown[]; + didSendViaMessagingTool?: boolean; successfulCronAdds?: number; didSendDeterministicApprovalPrompt?: boolean; }): boolean { return ( + params.didSendViaMessagingTool === true || hasSuccessfulSourceReplyDelivery(params) || (params.successfulCronAdds ?? 0) > 0 || params.didSendDeterministicApprovalPrompt === true @@ -1953,6 +1955,7 @@ export async function runReplyAgent(params: { messagingToolSentTexts: runResult.messagingToolSentTexts, messagingToolSentMediaUrls: runResult.messagingToolSentMediaUrls, messagingToolSentTargets: runResult.messagingToolSentTargets, + didSendViaMessagingTool: runResult.didSendViaMessagingTool, successfulCronAdds: runResult.successfulCronAdds, didSendDeterministicApprovalPrompt: runResult.didSendDeterministicApprovalPrompt, }); diff --git a/src/channels/plugins/types.core.ts b/src/channels/plugins/types.core.ts index 12b5ba127124..f8496f610219 100644 --- a/src/channels/plugins/types.core.ts +++ b/src/channels/plugins/types.core.ts @@ -765,6 +765,8 @@ export type ChannelMessageActionAdapter = { action: ChannelMessageActionName; toolContext?: ChannelThreadingToolContext; }) => boolean; + /** Return true when a provider-native tool invocation has a visible or destructive side effect. */ + isToolDeliveryAction?: (params: { args: Record }) => boolean; extractToolSend?: (params: { args: Record }) => ChannelToolSend | null; /** Recover the actual resolved send route from a successful action result. */ extractToolSendResult?: (params: { diff --git a/src/config/sessions/types.ts b/src/config/sessions/types.ts index 302f70aa4b1e..f96d0f4e1396 100644 --- a/src/config/sessions/types.ts +++ b/src/config/sessions/types.ts @@ -51,6 +51,7 @@ export type CliSessionBinding = { authEpoch?: string; authEpochVersion?: number; extraSystemPromptHash?: string; + messageToolPolicyHash?: string; promptToolNamesHash?: string; cwdHash?: string; mcpConfigHash?: string; diff --git a/src/gateway/mcp-http.handlers.ts b/src/gateway/mcp-http.handlers.ts index 67bf2559a2fa..9008ec11d1b8 100644 --- a/src/gateway/mcp-http.handlers.ts +++ b/src/gateway/mcp-http.handlers.ts @@ -47,6 +47,13 @@ export async function handleMcpJsonRpc(params: { toolSchema: McpToolSchemaEntry[]; hookContext?: HookContext; signal?: AbortSignal; + onToolCallResult?: (call: { + toolName: string; + args: Record; + result?: unknown; + isError: boolean; + }) => void; + onToolCallPrepared?: (call: { toolName: string; args: Record }) => void; }): Promise { const { id, method, params: methodParams } = params.message; @@ -97,6 +104,19 @@ export async function handleMcpJsonRpc(params: { }); } const toolCallId = `mcp-${crypto.randomUUID()}`; + let executedToolArgs = toolArgs; + const reportToolCallResult = (result: unknown, isError: boolean) => { + try { + params.onToolCallResult?.({ + toolName, + args: executedToolArgs, + result, + isError, + }); + } catch { + // Observability callbacks must never alter the tool result returned to the MCP client. + } + }; try { // Gateway before-tool hooks still run for loopback MCP calls so policy // and audit behavior matches native tool calls from normal chat runs. @@ -113,12 +133,20 @@ export async function handleMcpJsonRpc(params: { isError: true, }); } + executedToolArgs = hookResult.params as Record; + try { + params.onToolCallPrepared?.({ toolName, args: executedToolArgs }); + } catch { + // Observability callbacks must never alter the tool result returned to the MCP client. + } const result = await tool.execute(toolCallId, hookResult.params, params.signal); + reportToolCallResult(result, false); return jsonRpcResult(id, { content: normalizeToolCallContent(result), isError: false, }); } catch (error) { + reportToolCallResult(error, true); const message = formatErrorMessage(error); return jsonRpcResult(id, { content: [{ type: "text", text: message || "tool execution failed" }], diff --git a/src/gateway/mcp-http.loopback-runtime.ts b/src/gateway/mcp-http.loopback-runtime.ts index 6b5e7e906411..f78706d40500 100644 --- a/src/gateway/mcp-http.loopback-runtime.ts +++ b/src/gateway/mcp-http.loopback-runtime.ts @@ -5,7 +5,317 @@ type McpLoopbackRuntime = { nonOwnerToken: string; }; +export type McpLoopbackToolCallResult = { + toolName: string; + args: Record; + result?: unknown; + isError: boolean; +}; + +export type McpLoopbackToolCallStart = Pick; + +type McpLoopbackToolCallCapture = { + onRequestStart?: () => void; + onRequestClassified?: () => void; + onRequestFinish?: () => void; + onToolCallStart?: (call: McpLoopbackToolCallStart) => void; + onToolCallUpdate?: (calls: { + previous: McpLoopbackToolCallStart; + current: McpLoopbackToolCallStart; + }) => void; + onToolCallFinish?: (call: McpLoopbackToolCallStart, state: { prepared: boolean }) => void; + onToolCallResult: (call: McpLoopbackToolCallResult) => void; + inFlight: number; + activityVersion: number; + activityWaiters: Set<() => void>; +}; + +export type McpLoopbackRequestCaptureHandle = { + capture: McpLoopbackToolCallCapture; + classified: boolean; + finished: boolean; +}; + +export type McpLoopbackToolCallCaptureHandle = { + capture: McpLoopbackToolCallCapture; + call: McpLoopbackToolCallStart; + prepared: boolean; + finished: boolean; +}; + let activeRuntime: McpLoopbackRuntime | undefined; +const toolCallCaptures = new Map(); + +function deleteMcpLoopbackToolCallCapture(captureKey: string): void { + const capture = toolCallCaptures.get(captureKey); + if (!capture) { + return; + } + toolCallCaptures.delete(captureKey); + for (const resolve of capture.activityWaiters) { + resolve(); + } + capture.activityWaiters.clear(); +} + +function notifyMcpLoopbackToolCallCaptureActivity(capture: McpLoopbackToolCallCapture): void { + capture.activityVersion += 1; + for (const resolve of capture.activityWaiters) { + resolve(); + } + capture.activityWaiters.clear(); +} + +/** Start loopback tool-call result capture for one serialized CLI invocation. */ +export function beginMcpLoopbackToolCallCapture(params: { + captureKey: string; + onRequestStart?: () => void; + onRequestClassified?: () => void; + onRequestFinish?: () => void; + onToolCallStart?: (call: McpLoopbackToolCallStart) => void; + onToolCallUpdate?: (calls: { + previous: McpLoopbackToolCallStart; + current: McpLoopbackToolCallStart; + }) => void; + onToolCallFinish?: (call: McpLoopbackToolCallStart, state: { prepared: boolean }) => void; + onToolCallResult: (call: McpLoopbackToolCallResult) => void; +}): void { + const captureKey = params.captureKey.trim(); + if (!captureKey) { + return; + } + toolCallCaptures.set(captureKey, { + onRequestStart: params.onRequestStart, + onRequestClassified: params.onRequestClassified, + onRequestFinish: params.onRequestFinish, + onToolCallStart: params.onToolCallStart, + onToolCallUpdate: params.onToolCallUpdate, + onToolCallFinish: params.onToolCallFinish, + onToolCallResult: params.onToolCallResult, + inFlight: 0, + activityVersion: 0, + activityWaiters: new Set(), + }); +} + +/** Bind an authenticated HTTP request to the active capture generation before reading its body. */ +export function markMcpLoopbackRequestStarted( + captureKey: string | undefined, +): McpLoopbackRequestCaptureHandle | undefined { + const normalizedKey = captureKey?.trim() ?? ""; + if (!normalizedKey) { + return undefined; + } + const capture = toolCallCaptures.get(normalizedKey); + if (!capture) { + return undefined; + } + capture.inFlight += 1; + notifyMcpLoopbackToolCallCaptureActivity(capture); + try { + capture.onRequestStart?.(); + } catch { + // Delivery observation is diagnostic state; it must not alter request handling. + } + return { capture, classified: false, finished: false }; +} + +/** Mark a request body as parsed so it no longer represents an unknown possible send. */ +export function markMcpLoopbackRequestClassified( + captureHandle: McpLoopbackRequestCaptureHandle | undefined, +): void { + if (!captureHandle || captureHandle.classified || captureHandle.finished) { + return; + } + captureHandle.classified = true; + try { + captureHandle.capture.onRequestClassified?.(); + } catch { + // Delivery observation is diagnostic state; it must not alter request handling. + } +} + +/** Mark an authenticated request as settled and wake capture drains. */ +export function markMcpLoopbackRequestFinished( + captureHandle: McpLoopbackRequestCaptureHandle | undefined, +): void { + if (!captureHandle || captureHandle.finished) { + return; + } + markMcpLoopbackRequestClassified(captureHandle); + captureHandle.finished = true; + const { capture } = captureHandle; + try { + capture.onRequestFinish?.(); + } catch { + // Delivery observation is diagnostic state; it must not alter request handling. + } + capture.inFlight = Math.max(0, capture.inFlight - 1); + notifyMcpLoopbackToolCallCaptureActivity(capture); +} + +/** Mark a captured loopback tool call as in flight. */ +export function markMcpLoopbackToolCallStarted(params: { + captureKey?: string; + requestCaptureHandle?: McpLoopbackRequestCaptureHandle; + toolName: string; + args: Record; +}): McpLoopbackToolCallCaptureHandle | undefined { + const toolName = params.toolName.trim(); + if (!toolName || params.requestCaptureHandle?.finished) { + return undefined; + } + const captureKey = params.captureKey?.trim() ?? ""; + const capture = params.requestCaptureHandle?.capture ?? toolCallCaptures.get(captureKey); + if (!capture) { + return undefined; + } + const call = { toolName, args: params.args }; + capture.inFlight += 1; + notifyMcpLoopbackToolCallCaptureActivity(capture); + try { + capture.onToolCallStart?.(call); + } catch { + // Delivery observation is diagnostic state; it must not alter tool execution. + } + return { capture, call, prepared: false, finished: false }; +} + +/** Update an admitted call with the final arguments produced by gateway hooks. */ +export function updateMcpLoopbackToolCallCapture( + captureHandle: McpLoopbackToolCallCaptureHandle | undefined, + call: McpLoopbackToolCallStart, +): void { + if (!captureHandle || captureHandle.finished) { + return; + } + const previous = captureHandle.call; + captureHandle.call = call; + captureHandle.prepared = true; + try { + captureHandle.capture.onToolCallUpdate?.({ previous, current: call }); + } catch { + // Delivery observation is diagnostic state; it must not alter tool execution. + } +} + +/** Report a completed call without letting observer failures alter tool execution. */ +export function recordMcpLoopbackToolCallResult(params: { + captureHandle: McpLoopbackToolCallCaptureHandle; + toolName: string; + args: Record; + result?: unknown; + isError: boolean; +}): void { + const toolName = params.toolName.trim(); + if (!toolName) { + return; + } + try { + params.captureHandle.capture.onToolCallResult({ + toolName, + args: params.args, + result: params.result, + isError: params.isError, + }); + } catch { + // Delivery observation is diagnostic state; it must not turn a successful tool call into error. + } +} + +/** Mark a captured loopback tool call as settled and wake idle drains. */ +export function markMcpLoopbackToolCallFinished( + captureHandle: McpLoopbackToolCallCaptureHandle | undefined, +): void { + if (!captureHandle || captureHandle.finished) { + return; + } + captureHandle.finished = true; + const { capture } = captureHandle; + try { + capture.onToolCallFinish?.(captureHandle.call, { prepared: captureHandle.prepared }); + } catch { + // Delivery observation is diagnostic state; it must not alter tool execution. + } + capture.inFlight = Math.max(0, capture.inFlight - 1); + notifyMcpLoopbackToolCallCaptureActivity(capture); +} + +async function waitForMcpLoopbackToolCallCaptureActivity( + capture: McpLoopbackToolCallCapture, + timeoutMs: number, +): Promise { + return await new Promise((resolve) => { + let settled = false; + const finish = (active: boolean) => { + if (settled) { + return; + } + settled = true; + clearTimeout(timer); + capture.activityWaiters.delete(resolveActivity); + resolve(active); + }; + const resolveActivity = () => finish(true); + const timer = setTimeout(() => finish(false), Math.max(0, timeoutMs)); + timer.unref?.(); + capture.activityWaiters.add(resolveActivity); + }); +} + +/** Wait for admitted calls to settle and for a quiet request-admission grace. */ +export async function waitForMcpLoopbackToolCallCaptureIdle( + captureKey: string, + options: { + timeoutMs: number; + admissionGraceMs: number; + }, +): Promise { + const normalizedKey = captureKey.trim(); + const capture = toolCallCaptures.get(normalizedKey); + if (!capture) { + return true; + } + const deadline = Date.now() + Math.max(0, options.timeoutMs); + while (toolCallCaptures.get(normalizedKey) === capture) { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) { + return false; + } + if (capture.inFlight > 0) { + await waitForMcpLoopbackToolCallCaptureActivity(capture, remainingMs); + continue; + } + const admissionGraceMs = Math.max(0, options.admissionGraceMs); + if (admissionGraceMs === 0) { + return true; + } + const activityVersion = capture.activityVersion; + const quietWaitMs = Math.min(admissionGraceMs, remainingMs); + const sawActivity = await waitForMcpLoopbackToolCallCaptureActivity(capture, quietWaitMs); + if ( + !sawActivity && + quietWaitMs === admissionGraceMs && + capture.inFlight === 0 && + capture.activityVersion === activityVersion + ) { + return true; + } + } + return true; +} + +/** Clear an unfinished invocation capture. Attempt keys are unique per CLI execution. */ +export function clearMcpLoopbackToolCallCapture(captureKey: string): void { + deleteMcpLoopbackToolCallCapture(captureKey.trim()); +} + +/** Clear transient capture state between isolated tests. */ +export function clearMcpLoopbackToolCallCapturesForTest(): void { + for (const captureKey of toolCallCaptures.keys()) { + deleteMcpLoopbackToolCallCapture(captureKey); + } +} /** Return a copy of the active loopback runtime, if one has been installed. */ export function getActiveMcpLoopbackRuntime(): McpLoopbackRuntime | undefined { @@ -53,6 +363,7 @@ export function createMcpLoopbackServerConfig(port: number) { "x-openclaw-source-reply-delivery-mode": "${OPENCLAW_MCP_SOURCE_REPLY_DELIVERY_MODE}", "x-openclaw-require-explicit-message-target": "${OPENCLAW_MCP_REQUIRE_EXPLICIT_MESSAGE_TARGET}", + "x-openclaw-cli-capture-key": "${OPENCLAW_MCP_CLI_CAPTURE_KEY}", }, }, }, diff --git a/src/gateway/mcp-http.request.ts b/src/gateway/mcp-http.request.ts index ef8ca8c41f2b..e08271dfab5b 100644 --- a/src/gateway/mcp-http.request.ts +++ b/src/gateway/mcp-http.request.ts @@ -354,6 +354,10 @@ export function resolveMcpHttpBodyTimeoutMs(): number { return readPositiveIntEnv("OPENCLAW_MCP_LOOPBACK_BODY_TIMEOUT_MS", DEFAULT_MCP_BODY_TIMEOUT_MS); } +export function resolveMcpCliCaptureKey(req: IncomingMessage): string | undefined { + return normalizeOptionalString(getHeader(req, "x-openclaw-cli-capture-key")); +} + export function resolveMcpRequestContext( req: IncomingMessage, cfg: OpenClawConfig, diff --git a/src/gateway/mcp-http.test.ts b/src/gateway/mcp-http.test.ts index 46a0f1bb3140..129cebc355a5 100644 --- a/src/gateway/mcp-http.test.ts +++ b/src/gateway/mcp-http.test.ts @@ -106,6 +106,15 @@ import { ensureMcpLoopbackServer, startMcpLoopbackServer, } from "./mcp-http.js"; +import { + beginMcpLoopbackToolCallCapture, + clearMcpLoopbackToolCallCapture, + clearMcpLoopbackToolCallCapturesForTest, + markMcpLoopbackToolCallFinished, + markMcpLoopbackToolCallStarted, + recordMcpLoopbackToolCallResult, + waitForMcpLoopbackToolCallCaptureIdle, +} from "./mcp-http.loopback-runtime.js"; import { McpLoopbackToolCache } from "./mcp-http.runtime.js"; let server: Awaited> | undefined; @@ -554,6 +563,7 @@ function buildMockMcpToolSchema(tools: MockGatewayTool[]) { } beforeEach(() => { + clearMcpLoopbackToolCallCapturesForTest(); resolveGatewayScopedToolsMock.mockClear(); runBeforeToolCallHookMock.mockClear(); runBeforeToolCallHookMock.mockImplementation( @@ -938,6 +948,358 @@ describe("mcp loopback server", () => { expectMcpResultText(payload, "CRON_EXECUTED"); }); + it("captures only successful calls with an explicit CLI capture key", async () => { + const captureKey = "google-gemini-cli"; + const captured: Array<{ toolName: string; args: Record }> = []; + const startedTargets: unknown[] = []; + const finishedTargets: unknown[] = []; + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallStart: ({ args }) => startedTargets.push(args.target), + onToolCallFinish: ({ args }) => finishedTargets.push(args.target), + onToolCallResult: ({ toolName, args }) => { + if (toolName === "message" && args.action === "send") { + captured.push({ toolName, args }); + } + }, + }); + const { runtime } = await startLoopbackServerForTest(); + + expect( + ( + await sendLoopbackToolCall({ + token: runtime.ownerToken, + name: "message", + args: { action: "send", target: "chat123", message: "sent" }, + headers: { "x-openclaw-cli-capture-key": captureKey }, + }) + ).status, + ).toBe(200); + + runBeforeToolCallHookMock.mockResolvedValueOnce({ + blocked: true, + reason: "blocked for test", + }); + expect( + ( + await sendLoopbackToolCall({ + token: runtime.ownerToken, + name: "message", + args: { action: "send", target: "blocked", message: "not sent" }, + headers: { "x-openclaw-cli-capture-key": captureKey }, + }) + ).status, + ).toBe(200); + + expect( + ( + await sendLoopbackToolCall({ + token: runtime.ownerToken, + name: "message", + args: { action: "send", target: "implicit-main", message: "not captured" }, + }) + ).status, + ).toBe(200); + + expect(captured).toEqual([ + expect.objectContaining({ + toolName: "message", + args: { action: "send", target: "chat123", message: "sent" }, + }), + ]); + expect(startedTargets).toEqual(["chat123", "blocked"]); + expect(finishedTargets).toEqual(["chat123", "blocked"]); + }); + + it("updates capture accounting with hook-rewritten tool arguments", async () => { + const captureKey = "hook-rewritten-send"; + const updatedCalls = vi.fn(); + const finishedCalls = vi.fn(); + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallUpdate: updatedCalls, + onToolCallFinish: finishedCalls, + onToolCallResult: vi.fn(), + }); + runBeforeToolCallHookMock.mockResolvedValueOnce({ + blocked: false, + params: { + action: "send", + target: "rewritten-target", + message: "rewritten send", + }, + }); + const { runtime } = await startLoopbackServerForTest(); + + await sendLoopbackToolCall({ + token: runtime.ownerToken, + name: "message", + args: { action: "react", target: "original-target" }, + headers: { "x-openclaw-cli-capture-key": captureKey }, + }); + + expect(updatedCalls).toHaveBeenCalledWith({ + previous: { + toolName: "message", + args: { action: "react", target: "original-target" }, + }, + current: { + toolName: "message", + args: { + action: "send", + target: "rewritten-target", + message: "rewritten send", + }, + }, + }); + expect(finishedCalls).toHaveBeenCalledWith( + { + toolName: "message", + args: { + action: "send", + target: "rewritten-target", + message: "rewritten send", + }, + }, + { prepared: true }, + ); + }); + + it("reports oversized successful calls without retaining their payloads", () => { + const captureKey = "oversized-capture"; + const captured = vi.fn(); + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallResult: captured, + }); + + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey, + toolName: "message", + args: { action: "send", target: "chat123" }, + }); + if (!captureHandle) { + throw new Error("Expected active MCP capture"); + } + recordMcpLoopbackToolCallResult({ + captureHandle, + toolName: "message", + args: { action: "send", target: "chat123" }, + result: { content: "x".repeat(20 * 1024) }, + isError: false, + }); + markMcpLoopbackToolCallFinished(captureHandle); + + expect(captured).toHaveBeenCalledWith( + expect.objectContaining({ + toolName: "message", + args: { action: "send", target: "chat123" }, + }), + ); + }); + + it("keeps admitted calls bound to their original capture generation", () => { + const captureKey = "generation-bound-capture"; + const firstCapture = vi.fn(); + const secondCapture = vi.fn(); + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallResult: firstCapture, + }); + const firstHandle = markMcpLoopbackToolCallStarted({ + captureKey, + toolName: "message", + args: { action: "send", target: "first-turn" }, + }); + if (!firstHandle) { + throw new Error("Expected first MCP capture generation"); + } + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallResult: secondCapture, + }); + + recordMcpLoopbackToolCallResult({ + captureHandle: firstHandle, + toolName: "message", + args: { action: "send", target: "first-turn" }, + result: { status: "sent" }, + isError: false, + }); + markMcpLoopbackToolCallFinished(firstHandle); + + expect(firstCapture).toHaveBeenCalledOnce(); + expect(secondCapture).not.toHaveBeenCalled(); + }); + + it("binds slow request bodies to their capture generation at header acceptance", async () => { + const captureKey = "slow-request-generation"; + const requestClassified = vi.fn(); + const requestStarted = vi.fn(); + const captured = vi.fn(); + let resolveRequestStarted: (() => void) | undefined; + const requestStartedPromise = new Promise((resolve) => { + resolveRequestStarted = resolve; + }); + beginMcpLoopbackToolCallCapture({ + captureKey, + onRequestStart: () => { + requestStarted(); + resolveRequestStarted?.(); + }, + onRequestClassified: requestClassified, + onToolCallResult: captured, + }); + const { runtime, port } = await startLoopbackServerForTest(); + const responsePromise = new Promise<{ status: number | undefined; body: string }>( + (resolve, reject) => { + const req = request( + { + hostname: "127.0.0.1", + port, + path: "/mcp", + method: "POST", + headers: { + authorization: `Bearer ${runtime.ownerToken}`, + "content-type": "application/json", + "transfer-encoding": "chunked", + "x-openclaw-cli-capture-key": captureKey, + }, + }, + (res) => { + let body = ""; + res.setEncoding("utf8"); + res.on("data", (chunk) => { + body += chunk; + }); + res.on("end", () => resolve({ status: res.statusCode, body })); + }, + ); + req.on("error", reject); + req.flushHeaders(); + void requestStartedPromise.then(() => { + clearMcpLoopbackToolCallCapture(captureKey); + req.end(mcpToolCallBody("message", { action: "send", target: "late-body" })); + }); + }, + ); + + await requestStartedPromise; + expect(requestStarted).toHaveBeenCalledOnce(); + expect(requestClassified).not.toHaveBeenCalled(); + const response = await responsePromise; + + expect(response.status).toBe(200); + expect(requestClassified).toHaveBeenCalledOnce(); + expect(captured).toHaveBeenCalledWith( + expect.objectContaining({ + toolName: "message", + args: { action: "send", target: "late-body" }, + isError: false, + }), + ); + }); + + it("waits through a quiet admission grace before clearing a failed-turn capture", async () => { + const captureKey = "admission-grace"; + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallResult: vi.fn(), + }); + const idlePromise = waitForMcpLoopbackToolCallCaptureIdle(captureKey, { + timeoutMs: 500, + admissionGraceMs: 40, + }); + await new Promise((resolve) => { + setTimeout(resolve, 10); + }); + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey, + toolName: "message", + args: { action: "send", target: "late-admission" }, + }); + if (!captureHandle) { + throw new Error("Expected late MCP capture admission"); + } + setTimeout(() => markMcpLoopbackToolCallFinished(captureHandle), 10); + + await expect(idlePromise).resolves.toBe(true); + }); + + it("keeps capture observer errors from changing tool success", async () => { + const captureKey = "throwing-observer"; + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallResult: () => { + throw new Error("observer failed"); + }, + }); + const { runtime } = await startLoopbackServerForTest(); + + const response = await sendLoopbackToolCall({ + token: runtime.ownerToken, + name: "message", + args: { action: "send", target: "chat123", message: "sent" }, + headers: { "x-openclaw-cli-capture-key": captureKey }, + }); + + expect(response.status).toBe(200); + const payload = await readMcpPayload(response); + expect(payload.result?.isError).toBe(false); + }); + + it("captures partial-delivery errors before returning the tool failure", async () => { + const captureKey = "partial-delivery"; + const captured = vi.fn(); + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallResult: captured, + }); + mockScopedTools([ + makeMessageTool({ + execute: async () => { + throw Object.assign(new Error("second chunk failed"), { sentBeforeError: true }); + }, + }), + ]); + const { runtime } = await startLoopbackServerForTest(); + + const response = await sendLoopbackToolCall({ + token: runtime.ownerToken, + name: "message", + args: { action: "send", target: "chat123", message: "sent partly" }, + headers: { "x-openclaw-cli-capture-key": captureKey }, + }); + + const payload = await readMcpPayload(response); + expect(payload.result?.isError).toBe(true); + expect(captured).toHaveBeenCalledWith( + expect.objectContaining({ + toolName: "message", + isError: true, + result: expect.objectContaining({ sentBeforeError: true }), + }), + ); + }); + + it("ignores calls after a capture is cleared", () => { + const captureKey = "cleared-capture"; + const captured = vi.fn(); + beginMcpLoopbackToolCallCapture({ + captureKey, + onToolCallResult: captured, + }); + clearMcpLoopbackToolCallCapturesForTest(); + const captureHandle = markMcpLoopbackToolCallStarted({ + captureKey, + toolName: "message", + args: { action: "send", target: "old-turn" }, + }); + + expect(captureHandle).toBeUndefined(); + expect(captured).not.toHaveBeenCalled(); + }); + it("calls healthy tools when an earlier loopback tool name is unreadable", async () => { const messageExecute = vi.fn(async () => ({ content: [{ type: "text", text: "MESSAGE_EXECUTED" }], @@ -1289,6 +1651,9 @@ describe("createMcpLoopbackServerConfig", () => { expect( config.mcpServers?.openclaw?.headers?.["x-openclaw-require-explicit-message-target"], ).toBe("${OPENCLAW_MCP_REQUIRE_EXPLICIT_MESSAGE_TARGET}"); + expect(config.mcpServers?.openclaw?.headers?.["x-openclaw-cli-capture-key"]).toBe( + "${OPENCLAW_MCP_CLI_CAPTURE_KEY}", + ); expect(config.mcpServers?.openclaw?.headers).not.toHaveProperty("x-openclaw-sender-is-owner"); }); diff --git a/src/gateway/mcp-http.ts b/src/gateway/mcp-http.ts index 7d8b70be888d..f95ac3d68c59 100644 --- a/src/gateway/mcp-http.ts +++ b/src/gateway/mcp-http.ts @@ -14,13 +14,21 @@ import { logDebug, logWarn } from "../logger.js"; import { handleMcpJsonRpc } from "./mcp-http.handlers.js"; import { clearActiveMcpLoopbackRuntimeByOwnerToken, + markMcpLoopbackRequestClassified, + markMcpLoopbackRequestFinished, + markMcpLoopbackRequestStarted, + markMcpLoopbackToolCallFinished, + markMcpLoopbackToolCallStarted, + recordMcpLoopbackToolCallResult, setActiveMcpLoopbackRuntime, + updateMcpLoopbackToolCallCapture, } from "./mcp-http.loopback-runtime.js"; import { jsonRpcError, type JsonRpcRequest } from "./mcp-http.protocol.js"; import { isMcpHttpBodyTooLargeError, isMcpHttpBodyTimeoutError, readMcpHttpBody, + resolveMcpCliCaptureKey, resolveMcpHttpBodyTimeoutMs, resolveMcpRequestContext, validateMcpLoopbackRequest, @@ -180,12 +188,40 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ return; } + // Bind the request before body parsing/tool resolution. A CLI may exit while + // an accepted request is still uploading, and retries must not outrun it. + const cliRequestCaptureHandle = markMcpLoopbackRequestStarted(resolveMcpCliCaptureKey(req)); const requestAbort = createRequestAbortSignal(req, res); void (async () => { let parsed: JsonRpcRequest | JsonRpcRequest[] | undefined; + let cliCaptureHandles: Array> = []; try { const body = await readMcpHttpBody(req, { timeoutMs: resolveMcpHttpBodyTimeoutMs() }); parsed = parseMcpJsonBody(body); + const messages = Array.isArray(parsed) ? parsed : [parsed]; + cliCaptureHandles = messages.map((message) => { + if ( + !cliRequestCaptureHandle || + !isJsonRpcRequest(message) || + message.method !== "tools/call" + ) { + return undefined; + } + const admittedToolName = + isRecord(message.params) && typeof message.params.name === "string" + ? message.params.name + : ""; + const toolArgs = + isRecord(message.params) && isRecord(message.params.arguments) + ? message.params.arguments + : {}; + return markMcpLoopbackToolCallStarted({ + requestCaptureHandle: cliRequestCaptureHandle, + toolName: admittedToolName, + args: toolArgs, + }); + }); + markMcpLoopbackRequestClassified(cliRequestCaptureHandle); const cfg = getRuntimeConfig(); const requestContext = resolveMcpRequestContext(req, cfg, auth); const scopedTools = toolCache.resolve({ @@ -203,7 +239,6 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ senderIsOwner: requestContext.senderIsOwner, }); - const messages = Array.isArray(parsed) ? parsed : [parsed]; logMcpLoopbackTraffic("request", { batchSize: messages.length, methods: messages.map((message) => @@ -216,24 +251,49 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ cronVisible: scopedTools.toolSchema.some((tool) => tool.name === "cron"), }); const responses: object[] = []; - for (const message of messages) { + for (const [messageIndex, message] of messages.entries()) { if (!isJsonRpcRequest(message)) { responses.push(jsonRpcError(readJsonRpcRequestId(message), -32600, "Invalid Request")); continue; } - const response = await handleMcpJsonRpc({ - message, - tools: scopedTools.tools, - toolSchema: scopedTools.toolSchema, - hookContext: { - agentId: scopedTools.agentId, - config: cfg, - sessionKey: requestContext.sessionKey, - }, - signal: requestAbort.signal, - }); + const cliCaptureHandle = cliCaptureHandles[messageIndex]; + let response: object | null; + try { + response = await handleMcpJsonRpc({ + message, + tools: scopedTools.tools, + toolSchema: scopedTools.toolSchema, + hookContext: { + agentId: scopedTools.agentId, + config: cfg, + sessionKey: requestContext.sessionKey, + }, + signal: requestAbort.signal, + onToolCallPrepared: cliCaptureHandle + ? ({ toolName: preparedToolName, args }) => { + updateMcpLoopbackToolCallCapture(cliCaptureHandle, { + toolName: preparedToolName, + args, + }); + } + : undefined, + onToolCallResult: cliCaptureHandle + ? ({ toolName: resultToolName, args, result, isError }) => { + recordMcpLoopbackToolCallResult({ + captureHandle: cliCaptureHandle, + toolName: resultToolName, + args, + result, + isError, + }); + } + : undefined, + }); + } finally { + markMcpLoopbackToolCallFinished(cliCaptureHandle); + } if (response !== null) { - const toolName = + const responseToolName = message.method === "tools/call" && isRecord(message.params) ? message.params.name : undefined; @@ -241,7 +301,7 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ isRecord(response) && isRecord(response.result) && response.result.isError === true; logMcpLoopbackTraffic("response", { method: message.method, - toolName: typeof toolName === "string" ? toolName : undefined, + toolName: typeof responseToolName === "string" ? responseToolName : undefined, isError, }); responses.push(response); @@ -285,6 +345,10 @@ export async function startMcpLoopbackServer(port = 0): Promise<{ } } finally { requestAbort.cleanup(); + for (const captureHandle of cliCaptureHandles) { + markMcpLoopbackToolCallFinished(captureHandle); + } + markMcpLoopbackRequestFinished(cliRequestCaptureHandle); } })(); }); diff --git a/src/infra/outbound/internal-source-reply.ts b/src/infra/outbound/internal-source-reply.ts new file mode 100644 index 000000000000..f46670c27067 --- /dev/null +++ b/src/infra/outbound/internal-source-reply.ts @@ -0,0 +1,120 @@ +// Internal source-reply policy is shared by message execution and CLI delivery capture. +import { + normalizeOptionalLowercaseString, + normalizeOptionalString, +} from "@openclaw/normalization-core/string-coerce"; +import type { SourceReplyDeliveryMode } from "../../auto-reply/get-reply-options.types.js"; +import type { ChannelThreadingToolContext } from "../../channels/plugins/types.public.js"; +import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { parseAgentSessionKey, parseThreadSessionSuffix } from "../../routing/session-key.js"; +import { INTERNAL_MESSAGE_CHANNEL, normalizeMessageChannel } from "../../utils/message-channel.js"; +import { resolveOutboundChannelPlugin } from "./channel-resolution.js"; +import { isConfiguredChannel, listConfiguredMessageChannels } from "./channel-selection.js"; + +type InternalSourceReplySinkInput = { + cfg: OpenClawConfig; + action: string; + toolContext?: ChannelThreadingToolContext; + sessionKey?: string; + sourceReplyDeliveryMode?: SourceReplyDeliveryMode; +}; + +const SESSION_DELIVERY_PEER_KINDS = new Set(["channel", "direct", "dm", "group"]); + +function hasExternalSessionDeliveryRoute(sessionKey: string | undefined): boolean { + const parsedThread = parseThreadSessionSuffix(sessionKey); + const baseSessionKey = parsedThread.baseSessionKey ?? sessionKey; + const parsed = parseAgentSessionKey(baseSessionKey); + if (!parsed) { + return false; + } + const parts = parsed.rest.split(":").filter(Boolean); + if (parts.length < 3) { + return false; + } + const channel = normalizeMessageChannel(parts[0]); + if (!channel || channel === INTERNAL_MESSAGE_CHANNEL) { + return false; + } + if (parts.length >= 4 && (parts[2] === "direct" || parts[2] === "dm")) { + return Boolean(parts.slice(3).join(":").trim()); + } + return ( + SESSION_DELIVERY_PEER_KINDS.has(parts[1] ?? "") && Boolean(parts.slice(2).join(":").trim()) + ); +} + +function hasExplicitRouteParam(params: Record): boolean { + for (const key of ["channel", "target", "to", "channelId"]) { + if (normalizeOptionalString(params[key])) { + return true; + } + } + return ( + Array.isArray(params.targets) && params.targets.some((value) => normalizeOptionalString(value)) + ); +} + +function hasCurrentSourceReplyContext(input: InternalSourceReplySinkInput): boolean { + const provider = normalizeOptionalLowercaseString(input.toolContext?.currentChannelProvider); + if (!provider) { + return false; + } + if (provider === INTERNAL_MESSAGE_CHANNEL) { + // The message tool replaces ambient webchat context with an external route + // encoded in the session key. Do not classify that route as a private sink. + return !hasExternalSessionDeliveryRoute(input.sessionKey); + } + const currentMessageId = input.toolContext?.currentMessageId; + return Boolean( + normalizeOptionalString(input.toolContext?.currentChannelId) || + normalizeOptionalString(input.toolContext?.currentMessagingTarget) || + normalizeOptionalString(input.toolContext?.currentThreadTs) || + (typeof currentMessageId === "number" && Number.isFinite(currentMessageId)) || + normalizeOptionalString(currentMessageId), + ); +} + +async function hasConfiguredCurrentSourceChannel( + input: InternalSourceReplySinkInput, +): Promise { + const provider = + normalizeMessageChannel(input.toolContext?.currentChannelProvider) ?? + normalizeOptionalLowercaseString(input.toolContext?.currentChannelProvider); + if (!provider || provider === INTERNAL_MESSAGE_CHANNEL) { + return false; + } + if (!isConfiguredChannel(input.cfg, provider)) { + return false; + } + if (!resolveOutboundChannelPlugin({ channel: provider, cfg: input.cfg, allowBootstrap: true })) { + return false; + } + const configuredChannels = await listConfiguredMessageChannels(input.cfg); + return configuredChannels.some((channel) => channel === provider); +} + +/** Return whether this send resolves to the private current-run source-reply sink. */ +export async function shouldUseInternalSourceReplySink( + input: InternalSourceReplySinkInput, + params: Record, +): Promise { + const hasImplicitCurrentSourceRoute = + input.action === "send" && + input.sourceReplyDeliveryMode === "message_tool_only" && + hasCurrentSourceReplyContext(input) && + Boolean(input.sessionKey?.trim()) && + !hasExplicitRouteParam(params); + if (!hasImplicitCurrentSourceRoute) { + return false; + } + if ( + !normalizeOptionalString(input.toolContext?.currentChannelId) && + !normalizeOptionalString(input.toolContext?.currentMessagingTarget) + ) { + return true; + } + // Configured current-source channels can infer the target and deliver through + // the normal plugin path; the sink is only the private fallback. + return !(await hasConfiguredCurrentSourceChannel(input)); +} diff --git a/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts b/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts index fe5085f2b7ff..7097e7a2cb0a 100644 --- a/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts +++ b/src/infra/outbound/message-action-runner.plugin-dispatch.test.ts @@ -710,6 +710,134 @@ describe("runMessageAction plugin dispatch", () => { ); }); + it("preserves gateway send receipts in broadcast results", async () => { + const gatewayPlugin = createGatewayActionPlugin({ + pluginId: "gatewaychat", + label: "Gateway Chat", + blurb: "Gateway Chat broadcast test plugin.", + actions: ["send"], + messaging: { + targetResolver: { + looksLikeId: () => true, + }, + }, + handleAction: vi.fn(async () => jsonResult({ ok: true })), + }); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "gatewaychat", + source: "test", + plugin: gatewayPlugin, + }, + ]), + ); + mocks.callGatewayLeastPrivilege.mockResolvedValue({ + ok: true, + messageId: "gw-broadcast-1", + }); + + const result = await runMessageAction({ + cfg: { + channels: { + gatewaychat: { + enabled: true, + }, + }, + } as OpenClawConfig, + action: "broadcast", + params: { + channel: "gatewaychat", + targets: ["user-123"], + message: "hello from broadcast", + }, + gateway: { + clientName: "cli", + mode: "cli", + }, + }); + + expect(result).toMatchObject({ + kind: "broadcast", + payload: { + results: [ + { + channel: "gatewaychat", + to: "user-123", + ok: true, + payload: { + ok: true, + messageId: "gw-broadcast-1", + }, + }, + ], + }, + }); + expect(mocks.executeSendAction).not.toHaveBeenCalled(); + }); + + it("preserves partial-delivery evidence from failed broadcast sends", async () => { + const gatewayPlugin = createGatewayActionPlugin({ + pluginId: "gatewaychat", + label: "Gateway Chat", + blurb: "Gateway Chat partial broadcast test plugin.", + actions: ["send"], + messaging: { + targetResolver: { + looksLikeId: () => true, + }, + }, + handleAction: vi.fn(async () => jsonResult({ ok: true })), + }); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "gatewaychat", + source: "test", + plugin: gatewayPlugin, + }, + ]), + ); + mocks.callGatewayLeastPrivilege.mockRejectedValue( + Object.assign(new Error("second payload failed"), { sentBeforeError: true }), + ); + + const result = await runMessageAction({ + cfg: { + channels: { + gatewaychat: { + enabled: true, + }, + }, + } as OpenClawConfig, + action: "broadcast", + params: { + channel: "gatewaychat", + targets: ["user-123"], + message: "hello from broadcast", + }, + gateway: { + clientName: "cli", + mode: "cli", + }, + }); + + expect(result).toMatchObject({ + kind: "broadcast", + payload: { + results: [ + { + channel: "gatewaychat", + to: "user-123", + ok: false, + sentBeforeError: true, + error: "second payload failed", + }, + ], + }, + }); + }); + it("preserves buffer-only send bytes for gateway-side materialization", async () => { const gatewayPlugin = createGatewayActionPlugin({ pluginId: "gatewaychat", diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts index 42a759d47528..0077a24e4276 100644 --- a/src/infra/outbound/message-action-runner.ts +++ b/src/infra/outbound/message-action-runner.ts @@ -44,7 +44,6 @@ import { stripFormattedReasoningMessage } from "../../shared/text/formatted-reas import { parseInlineDirectives } from "../../utils/directive-tags.js"; import { INTERNAL_MESSAGE_CHANNEL, - normalizeMessageChannel, type GatewayClientMode, type GatewayClientName, } from "../../utils/message-channel.js"; @@ -52,11 +51,11 @@ import { formatErrorMessage } from "../errors.js"; import { throwIfAborted } from "./abort.js"; import { resolveOutboundChannelPlugin } from "./channel-resolution.js"; import { - isConfiguredChannel, listConfiguredMessageChannels, resolveMessageChannelSelection, } from "./channel-selection.js"; import type { OutboundSendDeps } from "./deliver.js"; +import { shouldUseInternalSourceReplySink } from "./internal-source-reply.js"; import { normalizeMessageActionInput } from "./message-action-normalization.js"; import { collectActionMediaSourceHints, @@ -160,6 +159,8 @@ export type MessageActionRunResult = to: string; ok: boolean; error?: string; + sentBeforeError?: true; + payload?: unknown; result?: MessageSendResult; }>; }; @@ -524,17 +525,6 @@ function collectMessageAttachmentMediaHints(value: unknown): string[] { return mediaUrls; } -function hasExplicitRouteParam(params: Record): boolean { - for (const key of ["channel", "target", "to", "channelId"]) { - if (normalizeOptionalString(params[key])) { - return true; - } - } - return ( - Array.isArray(params.targets) && params.targets.some((value) => normalizeOptionalString(value)) - ); -} - function hasExplicitTargetParam(params: Record): boolean { for (const key of ["target", "to", "channelId"]) { if (normalizeOptionalString(params[key])) { @@ -627,65 +617,6 @@ function applyImplicitSourceReplySendPolicy( params.bestEffort = true; } -function hasCurrentSourceReplyContext(input: RunMessageActionParams): boolean { - const provider = normalizeOptionalLowercaseString(input.toolContext?.currentChannelProvider); - if (!provider) { - return false; - } - if (provider === INTERNAL_MESSAGE_CHANNEL) { - return true; - } - const currentMessageId = input.toolContext?.currentMessageId; - return Boolean( - normalizeOptionalString(input.toolContext?.currentChannelId) || - normalizeOptionalString(input.toolContext?.currentMessagingTarget) || - normalizeOptionalString(input.toolContext?.currentThreadTs) || - (typeof currentMessageId === "number" && Number.isFinite(currentMessageId)) || - normalizeOptionalString(currentMessageId), - ); -} - -async function hasConfiguredCurrentSourceChannel(input: RunMessageActionParams): Promise { - const provider = - normalizeMessageChannel(input.toolContext?.currentChannelProvider) ?? - normalizeOptionalLowercaseString(input.toolContext?.currentChannelProvider); - if (!provider || provider === INTERNAL_MESSAGE_CHANNEL) { - return false; - } - if (!isConfiguredChannel(input.cfg, provider)) { - return false; - } - if (!resolveOutboundChannelPlugin({ channel: provider, cfg: input.cfg, allowBootstrap: true })) { - return false; - } - const configuredChannels = await listConfiguredMessageChannels(input.cfg); - return configuredChannels.some((channel) => channel === provider); -} - -async function shouldUseInternalSourceReplySink( - input: RunMessageActionParams, - params: Record, -) { - const hasImplicitCurrentSourceRoute = - input.action === "send" && - input.sourceReplyDeliveryMode === "message_tool_only" && - hasCurrentSourceReplyContext(input) && - Boolean(input.sessionKey?.trim()) && - !hasExplicitRouteParam(params); - if (!hasImplicitCurrentSourceRoute) { - return false; - } - if ( - !normalizeOptionalString(input.toolContext?.currentChannelId) && - !normalizeOptionalString(input.toolContext?.currentMessagingTarget) - ) { - return true; - } - // Configured current-source channels can infer the target and deliver through - // the normal plugin path; the sink is only the private fallback. - return !(await hasConfiguredCurrentSourceChannel(input)); -} - async function runGatewayPluginMessageActionOrNull(params: { cfg: OpenClawConfig; params: Record; @@ -776,6 +707,8 @@ async function handleBroadcastAction( to: string; ok: boolean; error?: string; + sentBeforeError?: true; + payload?: unknown; result?: MessageSendResult; }> = []; const isAbortError = (err: unknown): boolean => err instanceof Error && err.name === "AbortError"; @@ -802,6 +735,7 @@ async function handleBroadcastAction( channel: targetChannel, to: resolved.to, ok: true, + payload: sendResult.kind === "send" ? sendResult.payload : undefined, result: sendResult.kind === "send" ? sendResult.sendResult : undefined, }); } catch (err) { @@ -813,6 +747,11 @@ async function handleBroadcastAction( to: target, ok: false, error: formatErrorMessage(err), + ...(err && + typeof err === "object" && + (err as { sentBeforeError?: unknown }).sentBeforeError === true + ? { sentBeforeError: true as const } + : {}), }); } } diff --git a/src/infra/outbound/message.test.ts b/src/infra/outbound/message.test.ts index 7e153681c1a7..5a2a62ed7b4c 100644 --- a/src/infra/outbound/message.test.ts +++ b/src/infra/outbound/message.test.ts @@ -486,6 +486,19 @@ describe("sendMessage", () => { expect(mocks.resolveRuntimePluginRegistry).not.toHaveBeenCalled(); }); + it("preserves suppressed direct-send status", async () => { + mocks.deliverOutboundPayloads.mockResolvedValueOnce([]); + + const result = await sendMessage({ + cfg: {}, + channel: "forum", + to: "123456", + content: "hidden", + }); + + expect(result.deliveryStatus).toBe("suppressed"); + }); + it("does not throw best-effort direct send failures", async () => { mocks.deliverOutboundPayloads.mockImplementationOnce(async (params: unknown) => { ( diff --git a/src/infra/outbound/message.ts b/src/infra/outbound/message.ts index c84b6e47dec9..5af26675107c 100644 --- a/src/infra/outbound/message.ts +++ b/src/infra/outbound/message.ts @@ -100,6 +100,7 @@ export type MessageSendResult = { mediaUrl: string | null; mediaUrls?: string[]; result?: OutboundDeliveryResult | { messageId: string }; + deliveryStatus?: "suppressed"; dryRun?: boolean; }; @@ -413,6 +414,7 @@ export async function sendMessage(params: MessageSendParams): Promise