fix(cli): address --message-file review findings

Joseff531 · Joseff531 · commit cf9bc0cd2e18 · 2026-05-11T06:02:28.000+02:00
diff --git a/docs/cli/message.md b/docs/cli/message.md
@@ -68,8 +68,8 @@ Name lookup:
 
 - `send`
   - Channels: WhatsApp/Telegram/Discord/Google Chat/Slack/Mattermost (plugin)/Signal/iMessage/Matrix/Microsoft Teams
-  - Required: `--target`, plus `--message`, `--media`, or `--presentation`
-  - Optional: `--media`, `--presentation`, `--delivery`, `--pin`, `--reply-to`, `--thread-id`, `--gif-playback`, `--force-document`, `--silent`
+  - Required: `--target`, plus `--message` or `--message-file`, `--media`, or `--presentation`
+  - Optional: `--message-file <path>` (read message body from a file; mutually exclusive with `--message`), `--media`, `--presentation`, `--delivery`, `--pin`, `--reply-to`, `--thread-id`, `--gif-playback`, `--force-document`, `--silent`
   - Shared presentation payloads: `--presentation` sends semantic blocks (`text`, `context`, `divider`, `buttons`, `select`) that core renders through the selected channel's declared capabilities. See [Message Presentation](/plugins/message-presentation).
   - Generic delivery preferences: `--delivery` accepts delivery hints such as `{ "pin": true }`; `--pin` is shorthand for pinned delivery when the channel supports it.
   - Telegram only: `--force-document` (send images and GIFs as documents to avoid Telegram compression)
diff --git a/src/infra/outbound/message-action-runner.send-validation.test.ts b/src/infra/outbound/message-action-runner.send-validation.test.ts
@@ -1,7 +1,7 @@
 import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
-import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { OpenClawConfig } from "../../config/config.js";
 import { setActivePluginRegistry } from "../../plugins/runtime.js";
 import { createTestRegistry } from "../../test-utils/channel-plugins.js";
@@ -12,6 +12,20 @@ import {
   workspaceTestPlugin,
 } from "./message-action-runner.test-helpers.js";
 
+// Spy on executeSendAction to capture the final message text that reaches the send layer.
+// Called through to the real implementation so all other test behaviour is unchanged.
+let _lastSendMessage: string | undefined;
+vi.mock("./outbound-send-service.js", async (importOriginal) => {
+  const real = await importOriginal<typeof import("./outbound-send-service.js")>();
+  return {
+    ...real,
+    executeSendAction: vi.fn(async (...args: Parameters<typeof real.executeSendAction>) => {
+      _lastSendMessage = args[0].message;
+      return real.executeSendAction(...args);
+    }),
+  };
+});
+
 describe("runMessageAction send validation", () => {
   beforeEach(() => {
     setActivePluginRegistry(
@@ -164,18 +178,20 @@ describe("runMessageAction send --message-file", () => {
     expect(result.kind).toBe("send");
   });
 
-  it("preserves multiline content and special characters without throwing", async () => {
+  it("preserves file content exactly — no trim and no literal-backslash-n expansion", async () => {
+    _lastSendMessage = undefined;
     const filePath = path.join(tmpDir, "report.txt");
-    await fs.writeFile(
-      filePath,
-      "line 1\nline 2\n```code block```\n$VAR {interpolation} `backtick`",
-      "utf8",
-    );
+    // Leading spaces prove no trimming; literal \n (backslash+n) proves no \\n→\n expansion.
+    // Inline --message would both trim and expand these, corrupting JSON or indented content.
+    await fs.writeFile(filePath, "  hello\\nworld", "utf8");
     const result = await runDrySend({
       cfg: workspaceConfig,
       actionParams: { channel: "workspace", target: "#C12345678", messageFile: filePath },
     });
     expect(result.kind).toBe("send");
+    expect(_lastSendMessage).toContain("  hello");
+    expect(_lastSendMessage).toContain("hello\\nworld");
+    expect(_lastSendMessage).not.toContain("hello\nworld");
   });
 
   it("rejects when both --message and --message-file are provided", async () => {
@@ -194,6 +210,22 @@ describe("runMessageAction send --message-file", () => {
     ).rejects.toThrow(/use --message or --message-file, not both/i);
   });
 
+  it("rejects when --message-file is combined with an empty --message", async () => {
+    const filePath = path.join(tmpDir, "msg.txt");
+    await fs.writeFile(filePath, "from file", "utf8");
+    await expect(
+      runDrySend({
+        cfg: workspaceConfig,
+        actionParams: {
+          channel: "workspace",
+          target: "#C12345678",
+          message: "",
+          messageFile: filePath,
+        },
+      }),
+    ).rejects.toThrow(/use --message or --message-file, not both/i);
+  });
+
   it("throws a clear error when the file does not exist", async () => {
     await expect(
       runDrySend({
diff --git a/src/infra/outbound/message-action-runner.ts b/src/infra/outbound/message-action-runner.ts
@@ -623,13 +623,13 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise<MessageActi
   const hasInteractive = hasInteractiveReplyBlocks(params.interactive);
   const caption = readStringParam(params, "caption", { allowEmpty: true }) ?? "";
   const messageFile = readStringParam(params, "messageFile", { trim: false });
-  if (messageFile !== null && messageFile !== undefined && messageFile !== "") {
-    if (readStringParam(params, "message", { allowEmpty: true })) {
+  let fileMessage: string | null = null;
+  if (messageFile) {
+    if (params.message !== undefined) {
       throw new Error("use --message or --message-file, not both");
     }
-    let fileContent: string;
     try {
-      fileContent = await fs.readFile(messageFile, "utf8");
+      fileMessage = await fs.readFile(messageFile, "utf8");
     } catch (err) {
       const code = (err as NodeJS.ErrnoException).code;
       if (code === "ENOENT") {
@@ -640,14 +640,15 @@ async function handleSendAction(ctx: ResolvedActionContext): Promise<MessageActi
       }
       throw new Error(`failed to read message file: ${messageFile}`, { cause: err });
     }
-    params.message = fileContent;
   }
   let message =
+    fileMessage ??
     readStringParam(params, "message", {
       required: !mediaHint && !hasPresentation && !hasInteractive,
       allowEmpty: true,
-    }) ?? "";
-  if (message.includes("\\n")) {
+    }) ??
+    "";
+  if (fileMessage === null && message.includes("\\n")) {
     message = message.replaceAll("\\n", "\n");
   }
   if (!message.trim() && caption.trim()) {
