ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-28 05:16:23 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(feishu): preserve direct-send attachments (#125664)

Browse Source 
fix(feishu): preserve direct-send attachments (#125664)

Promote supported attachment aliases at the Feishu send boundary.
Reject unsupported or malformed attachment intent instead of returning text-only success.
Preserve caption receipts when media upload fails.

Co-authored-by: SunnyShu0925 <shu.zongyu@xydigit.com>
Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
SunnyShu
2026-08-26 13:59:09 +08:00
committed by GitHub
parent 4f91fbc4a5
commit f94eb160fb
6 changed files with 1391 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
extensions/feishu/src/channel.test.ts
+631
View File
@@ -3,6 +3,7 @@ import { createRequireRecord } from "openclaw/plugin-sdk/test-fixtures";
import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../runtime-api.js";
import { feishuPlugin } from "./channel.js";
import { FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER } from "./outbound.js";
import { looksLikeFeishuId, normalizeFeishuTarget, resolveReceiveIdType } from "./targets.js";
describe("feishu target classification", () => {
@@ -821,6 +822,78 @@ describe("feishuPlugin actions", () => {
expect(fallbackArgs.mediaReadFile).toBe(legacyReadFile);
});
// Regression for #112244 (third-review P1): when a direct `send` attachment
// routes through the presentation-fallback path (card falls back to text),
// an upload failure must still surface as a visible error instead of an
// `ok:true` receipt for a text-only fallback. The action stamps the
// propagate marker on channelData.feishu so sendFeishuFallbackPayload
// re-throws; here we verify the marker is stamped and the failure rejects.
it("propagates a media-upload failure on the send presentation-fallback path instead of returning ok:true", async () => {
feishuOutboundSendPayloadMock.mockRejectedValueOnce(new Error("upload failed"));
const trustedReadFile = vi.fn(async () => Buffer.from("approved image"));
const mediaAccess = {
localRoots: ["/approved/workspace"],
workspaceDir: "/approved/workspace",
readFile: trustedReadFile,
};
// An oversized table falls outside the Feishu card envelope, so
// `presentationFellBack` is true and the direct send routes the attachment
// through sendPayload instead of the normal sendMedia branch.
const presentation = {
blocks: [
{
type: "table" as const,
caption: "Large pipeline",
headers: ["Account", "Stage"],
rows: Array.from({ length: 400 }, (_entry, index) => [
`account-${String(index)}-${"x".repeat(80)}`,
"Review",
]),
},
],
};
const rawCardText = JSON.stringify({
schema: "2.0",
body: { elements: [{ tag: "markdown", content: "Raw card JSON must stay hidden" }] },
});
await expect(
feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: `[Nexus] ${rawCardText}`,
presentation,
media: "pipeline.png",
},
cfg: {
...cfg,
channels: {
...cfg.channels,
feishu: { ...cfg.channels?.feishu, responsePrefix: "[Nexus]" },
},
},
accountId: undefined,
toolContext: {},
mediaAccess,
mediaLocalRoots: ["/approved/workspace"],
mediaReadFile: trustedReadFile,
} as never),
).rejects.toThrow("upload failed");
expect(feishuOutboundSendPayloadMock).toHaveBeenCalledTimes(1);
const fallbackArgs = requireRecord(
mockCallArg(feishuOutboundSendPayloadMock, 0, 0, "feishuOutbound.sendPayload"),
"fallback args",
);
const fallbackPayload = requireRecord(fallbackArgs.payload, "fallback payload");
const feishuChannelData = requireRecord(
requireRecord(fallbackPayload.channelData, "fallback channelData").feishu,
"fallback channelData.feishu",
);
expect(feishuChannelData[FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER]).toBe(true);
});
it("prefers structured presentation over raw card JSON text", async () => {
sendCardFeishuMock.mockResolvedValueOnce({ messageId: "om_card", chatId: "oc_group_1" });
@@ -1073,6 +1146,523 @@ describe("feishuPlugin actions", () => {
]);
});
// Regression for #112244: a direct Gateway `message.action send` may arrive
// with attachment aliases (path/filePath/fileUrl/image/mediaUrl) instead of
// the canonical `media` field. The Feishu handler must promote any alias to a
// canonical mediaUrl and deliver through sendMedia, rather than silently
// dropping the attachment on the text-only success branch.
it.each([
["path", "/tmp/script.py"],
["filePath", "/tmp/script.py"],
["fileUrl", "file:///tmp/script.py"],
["image", "/tmp/image.png"],
["mediaUrl", "/tmp/media.png"],
] as const)("promotes send attachment alias %s to sendMedia", async (key, value) => {
feishuOutboundSendMediaMock.mockResolvedValueOnce({
channel: "feishu",
messageId: "om_media",
details: { messageId: "om_media", chatId: "oc_group_1" },
});
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
[key]: value,
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
expect(feishuOutboundSendMediaMock).toHaveBeenCalledOnce();
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
const mediaArgs = requireRecord(
mockCallArg(feishuOutboundSendMediaMock, 0, 0, "feishuOutbound.sendMedia"),
"outbound args",
);
expect(mediaArgs.mediaUrl).toBe(value);
});
it("rejects unsupported buffer payload on send instead of text-only success", async () => {
await expect(
feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
buffer: "aGVsbG8=",
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never),
).rejects.toThrow(
"Feishu send supports media attachments through media, mediaUrl, path, filePath, fileUrl, image, mediaUrls, or attachments[] with one of those fields; buffer/base64 payloads are not supported.",
);
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(sendCardFeishuMock).not.toHaveBeenCalled();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
});
it("rejects multiple media attachments on send rather than dropping all but the first", async () => {
await expect(
feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
mediaUrls: ["/tmp/first.png", "/tmp/second.png"],
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never),
).rejects.toThrow("Feishu send supports a single media attachment.");
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
});
it.each([
["file_path", "/tmp/script.py"],
["media_url", "/tmp/media.png"],
["file_url", "file:///tmp/script.py"],
] as const)("promotes snake_case send attachment alias %s to sendMedia", async (key, value) => {
feishuOutboundSendMediaMock.mockResolvedValueOnce({
channel: "feishu",
messageId: "om_media",
details: { messageId: "om_media", chatId: "oc_group_1" },
});
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
[key]: value,
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
expect(feishuOutboundSendMediaMock).toHaveBeenCalledOnce();
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
const mediaArgs = requireRecord(
mockCallArg(feishuOutboundSendMediaMock, 0, 0, "feishuOutbound.sendMedia"),
"outbound args",
);
expect(mediaArgs.mediaUrl).toBe(value);
});
it("promotes media_urls snake_case array alias to sendMedia", async () => {
feishuOutboundSendMediaMock.mockResolvedValueOnce({
channel: "feishu",
messageId: "om_media",
details: { messageId: "om_media", chatId: "oc_group_1" },
});
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
media_urls: ["/tmp/report.md"],
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
expect(feishuOutboundSendMediaMock).toHaveBeenCalledOnce();
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
const mediaArgs = requireRecord(
mockCallArg(feishuOutboundSendMediaMock, 0, 0, "feishuOutbound.sendMedia"),
"outbound args",
);
expect(mediaArgs.mediaUrl).toBe("/tmp/report.md");
});
it("accepts a single string mediaUrls value instead of dropping it", async () => {
feishuOutboundSendMediaMock.mockResolvedValueOnce({
channel: "feishu",
messageId: "om_media",
details: { messageId: "om_media", chatId: "oc_group_1" },
});
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
mediaUrls: "/tmp/single.png",
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
expect(feishuOutboundSendMediaMock).toHaveBeenCalledOnce();
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
const mediaArgs = requireRecord(
mockCallArg(feishuOutboundSendMediaMock, 0, 0, "feishuOutbound.sendMedia"),
"outbound args",
);
expect(mediaArgs.mediaUrl).toBe("/tmp/single.png");
});
// Regression for #112244 (ClawSweeper P1): a valid nested
// `attachments[].mediaUrls` string list must be collected as a media
// candidate, not just validated for malformed entries. Without this the
// request would fall through to a text-only `ok:true` — the silent drop the
// PR removes.
it("accepts a nested attachments[].mediaUrls list instead of dropping it", async () => {
feishuOutboundSendMediaMock.mockResolvedValueOnce({
channel: "feishu",
messageId: "om_media",
details: { messageId: "om_media", chatId: "oc_group_1" },
});
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
attachments: [{ mediaUrls: ["/tmp/nested.png"] }],
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
expect(feishuOutboundSendMediaMock).toHaveBeenCalledOnce();
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
const mediaArgs = requireRecord(
mockCallArg(feishuOutboundSendMediaMock, 0, 0, "feishuOutbound.sendMedia"),
"outbound args",
);
expect(mediaArgs.mediaUrl).toBe("/tmp/nested.png");
});
// `attachments[].image` must be collected as a media candidate just like the
// top-level `image` alias. Without this the request would fall through to a
// text-only `ok:true` — the silent drop the PR removes.
it("accepts a nested attachments[].image alias instead of dropping it", async () => {
feishuOutboundSendMediaMock.mockResolvedValueOnce({
channel: "feishu",
messageId: "om_media_image",
details: { messageId: "om_media_image", chatId: "oc_group_1" },
});
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached image",
attachments: [{ image: "/tmp/nested-image.png" }],
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
expect(feishuOutboundSendMediaMock).toHaveBeenCalledOnce();
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
const mediaArgs = requireRecord(
mockCallArg(feishuOutboundSendMediaMock, 0, 0, "feishuOutbound.sendMedia"),
"outbound args",
);
expect(mediaArgs.mediaUrl).toBe("/tmp/nested-image.png");
});
it.each(["buffer", "base64"] as const)(
"rejects nested %s attachment payload on send instead of text-only success",
async (field) => {
await expect(
feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
attachments: [{ [field]: "aGVsbG8=", filename: "report.md" }],
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never),
).rejects.toThrow("buffer/base64 payloads are not supported");
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(sendCardFeishuMock).not.toHaveBeenCalled();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
},
);
it("rejects mixed supported and nested unsupported attachment payloads on send", async () => {
await expect(
feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
attachments: [
{ filePath: "/tmp/report.md" },
{ buffer: "aGVsbG8=", filename: "report-copy.md" },
],
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never),
).rejects.toThrow("buffer/base64 payloads are not supported");
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
});
// The linked issue's literal reproduction is `{ message, file: "/path" }`.
// `file` is an attachment-intent key the Feishu send path does not map to a
// media source, so it must fail visibly instead of returning ok:true on a
// text-only send (issue #112244 expected behavior).
it.each([
{ location: "top-level", params: { file: "/tmp/script.py", filename: "script.py" } },
{ location: "nested", params: { attachments: [{ file: "/tmp/script.py" }] } },
])(
"rejects $location `file` attachment intent on send instead of text-only success",
async ({ params }) => {
await expect(
feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "here is the script",
...params,
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never),
).rejects.toThrow("`file` attachment-intent parameter is not supported");
// No text-only send slips through with a false ok:true.
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(sendCardFeishuMock).not.toHaveBeenCalled();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
},
);
// Regression for the eighteenth-review P1 finding: a present NON-string value
// for an unsupported attachment-intent key (`file`/`buffer`/`base64`) used to
// be silently skipped (readFeishuStringParam returns undefined for objects),
// leaving no candidate and falling through to a text-only ok:true — the exact
// silent-drop this PR removes. Post-fix the resolver rejects the malformed
// intent before the text branch, at both the top level and inside attachments[].
it.each([
{
label: "top-level `file: {}`",
params: { file: {} },
expectedError: "`file` attachment-intent parameter is not supported",
},
{
label: "top-level `buffer: {}`",
params: { buffer: {} },
expectedError: "buffer/base64 payloads are not supported",
},
{
label: "top-level `base64: {}`",
params: { base64: {} },
expectedError: "buffer/base64 payloads are not supported",
},
{
label: "top-level `file: 42`",
params: { file: 42 },
expectedError: "`file` attachment-intent parameter is not supported",
},
{
label: "nested `attachments: [{ file: {} }]`",
params: { attachments: [{ file: {} }] },
expectedError: "`file` attachment-intent parameter is not supported",
},
{
label: "nested `attachments: [{ buffer: {} }]`",
params: { attachments: [{ buffer: {} }] },
expectedError: "buffer/base64 payloads are not supported",
},
])(
"rejects malformed (non-string) $label attachment intent on send instead of text-only success",
async ({ params, expectedError }) => {
await expect(
feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
...params,
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never),
).rejects.toThrow(expectedError);
// No text-only send slips through with a false ok:true.
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(sendCardFeishuMock).not.toHaveBeenCalled();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
},
);
it("ignores a blank `file` attachment intent field on send", async () => {
sendMessageFeishuMock.mockResolvedValueOnce({ messageId: "om_plain", chatId: "oc_group_1" });
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "plain text",
file: " ",
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
// A blank `file` is not an attachment-intent signal; with no media alias
// the send still takes the text-only success branch (unchanged).
expect(sendMessageFeishuMock).toHaveBeenCalledOnce();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
});
// Regression for the second-review P1 finding: a declared media source whose
// value is present but malformed (e.g. `media: {}` or `mediaUrls: [{}]`)
// expresses attachment intent the Feishu send path cannot represent. Treating
// such a value as absent recreates the silent text-only `ok:true` success this
// PR removes, so it must fail visibly before the text branch instead.
it.each([
{ location: "top-level scalar `media`", params: { media: {} } },
{ location: "top-level scalar `filePath`", params: { filePath: 42 } },
{ location: "top-level `mediaUrls` object", params: { mediaUrls: {} } },
{ location: "top-level `mediaUrls` array with non-string entry", params: { mediaUrls: [{}] } },
{
location: "top-level `mediaUrls` array with number entry",
params: { mediaUrls: ["/tmp/a.png", 7] },
},
{ location: "nested scalar `media`", params: { attachments: [{ media: {} }] } },
{
location: "nested `mediaUrls` with non-string entry",
params: { attachments: [{ mediaUrls: [{}] }] },
},
// A declared `attachments` field that is not an array (an object container
// whose nested media source is not a top-level alias) is an attachment
// intent the resolver cannot promote; treating it as absent recreates the
// silent text-only `ok:true` drop this PR removes (issue #112244, ClawSweeper P1).
{
location: "object `attachments` container",
params: { attachments: { media: "/tmp/a.png" } },
},
// A non-record array entry is a declared attachment intent the resolver
// cannot promote; skipping it silently would fall through to a text-only
// success (issue #112244, ClawSweeper P1).
{ location: "`attachments` array with null entry", params: { attachments: [null] } },
{ location: "`attachments` array with non-record entry", params: { attachments: [42] } },
])(
"rejects $location malformed media source on send instead of text-only success",
async ({ params }) => {
await expect(
feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "see attached",
...params,
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never),
).rejects.toThrow("a present malformed media source value is not supported");
// No text-only send slips through with a false ok:true.
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
expect(sendCardFeishuMock).not.toHaveBeenCalled();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
},
);
it.each([
{ location: "blank scalar `media`", params: { media: " " } },
{ location: "blank `mediaUrls` array entry", params: { mediaUrls: [" "] } },
{
location: "blank nested `media`",
params: { attachments: [{ media: " " }] },
},
])("ignores blank $location media source on send (not malformed)", async ({ params }) => {
sendMessageFeishuMock.mockResolvedValueOnce({ messageId: "om_plain", chatId: "oc_group_1" });
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "plain text",
...params,
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
// A blank string is a string, not malformed; with no usable media it
// still takes the text-only success branch (unchanged).
expect(sendMessageFeishuMock).toHaveBeenCalledOnce();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
});
it.each([
{ location: "top-level", params: { buffer: "", base64: " " } },
{
location: "nested",
params: { attachments: [{ buffer: "", base64: " " }] },
},
])("ignores blank $location attachment payload fields on send", async ({ params }) => {
sendMessageFeishuMock.mockResolvedValueOnce({ messageId: "om_plain", chatId: "oc_group_1" });
await feishuPlugin.actions?.handleAction?.({
action: "send",
params: {
to: "chat:oc_group_1",
message: "plain text",
...params,
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
// Blank buffer/base64 is not an unsupported payload intent; with no media
// alias the send still takes the text-only success branch (unchanged).
expect(sendMessageFeishuMock).toHaveBeenCalledOnce();
expect(feishuOutboundSendMediaMock).not.toHaveBeenCalled();
});
it.each(["send", "thread-reply"] as const)(
"preserves only trusted workspace media access for %s actions",
async (action) => {
@@ -1122,6 +1712,10 @@ describe("feishuPlugin actions", () => {
mediaLocalRoots: ["/legacy/workspace"],
mediaReadFile: legacyReadFile,
...(action === "thread-reply" ? { threadId: "om_parent" } : { replyToId: undefined }),
// Only the direct `send` action propagates media-upload failures;
// thread-reply keeps its existing fallback-text behavior for
// compatibility (issue #112244).
...(action === "send" ? { propagateMediaUploadFailure: true } : {}),
});
const outboundArgs = requireRecord(
mockCallArg(feishuOutboundSendMediaMock, 0, 0, "feishuOutbound.sendMedia"),
@@ -1133,6 +1727,43 @@ describe("feishuPlugin actions", () => {
},
);
// Regression for #112244 (round-5 P1): upload-failure propagation is scoped
// to `send` only; a thread-reply whose media upload fails must keep its
// existing fallback-text behavior (no throw), preserving compatibility.
it("does not propagate media-upload failure for thread-reply (keeps fallback)", async () => {
// sendMedia resolves to a text fallback result (the fallback path lives in
// the outbound adapter; here we verify the action does not throw and does
// not pass propagateMediaUploadFailure for thread-reply).
feishuOutboundSendMediaMock.mockResolvedValueOnce({
channel: "feishu",
messageId: "om_thread_fallback",
details: { messageId: "om_thread_fallback", chatId: "oc_group_1" },
});
const result = await feishuPlugin.actions?.handleAction?.({
action: "thread-reply",
params: {
to: "chat:oc_group_1",
message: "thread reply",
messageId: "om_parent",
// thread-reply reads only the canonical `media` field (send promotes
// aliases, thread-reply does not), so use `media` to reach sendMedia.
media: "/tmp/script.py",
},
cfg,
accountId: undefined,
toolContext: {},
mediaLocalRoots: ["/tmp"],
} as never);
const outboundArgs = requireRecord(
mockCallArg(feishuOutboundSendMediaMock, 0, 0, "feishuOutbound.sendMedia"),
"outbound args",
);
expect(outboundArgs.propagateMediaUploadFailure).toBeUndefined();
expect(resultDetails(result).messageId).toBe("om_thread_fallback");
});
it("passes asVoice through media sends", async () => {
feishuOutboundSendMediaMock.mockResolvedValueOnce({
channel: "feishu",
extensions/feishu/src/channel.ts
+310 -2
View File
@@ -89,6 +89,10 @@ import { feishuDoctor } from "./doctor.js";
import { chunkFeishuMarkdown } from "./markdown.js";
import { messageActionTargetAliases } from "./message-action-contract.js";
import { readNativeFeishuCardJson } from "./native-card.js";
import {
FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER,
type FeishuOutboundSendMedia,
} from "./outbound.js";
import { resolveFeishuGroupToolPolicy } from "./policy.js";
import {
assertFeishuCardWithinEnvelope,
@@ -122,6 +126,276 @@ function readFeishuMediaParam(params: Record<string, unknown>): string | undefin
return media.trim() ? media : undefined;
}
// Path-shaped attachment param aliases the message-tool schema declares. A
// direct Gateway `message.action send` may arrive with any of these instead of
// the canonical `media` field the Feishu handler reads; collect them so an
// attachment intent is promoted to a single canonical mediaUrl instead of being
// silently dropped on the text-only success branch (issue #112244).
const FEISHU_SEND_MEDIA_SOURCE_PARAM_KEYS = [
"media",
"mediaUrl",
"path",
"filePath",
"fileUrl",
"image",
] as const;
const FEISHU_ATTACHMENT_MEDIA_SOURCE_PARAM_KEYS = [
"media",
"mediaUrl",
"path",
"filePath",
"fileUrl",
"url",
"image",
] as const;
// Converts a camelCase param key to its snake_case spelling (e.g. filePath ->
// file_path, mediaUrl -> media_url). The Gateway action contract resolves both
// spellings before a message action reaches a channel, so a Feishu send may
// arrive with either; the resolver honors both to avoid silently dropping an
// attachment alias that uses the snake_case spelling (issue #112244).
function toFeishuSnakeCaseKey(key: string): string {
return key
.replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2")
.replace(/([a-z0-9])([A-Z])/g, "$1_$2")
.toLowerCase();
}
// Reads a raw param value, preferring the canonical (camelCase) key and
// falling back to its snake_case spelling when the canonical key is absent.
function readFeishuParam(params: Record<string, unknown>, key: string): unknown {
if (Object.hasOwn(params, key)) {
return params[key];
}
const snakeKey = toFeishuSnakeCaseKey(key);
return snakeKey === key || !Object.hasOwn(params, snakeKey) ? undefined : params[snakeKey];
}
function readFeishuStringParam(params: Record<string, unknown>, key: string): string | undefined {
const raw = readFeishuParam(params, key);
const value = typeof raw === "string" ? normalizeOptionalString(raw) : undefined;
return value ?? undefined;
}
function readFeishuStringArrayParam(params: Record<string, unknown>, key: string): string[] {
const raw = readFeishuParam(params, key);
// A single string is accepted in place of an array so a caller that wrote
// `mediaUrls: "/tmp/a.png"` instead of `["/tmp/a.png"]` is not silently
// dropped on the text-only success branch.
if (Array.isArray(raw)) {
const normalized: string[] = [];
for (const entry of raw) {
const trimmed = typeof entry === "string" ? normalizeOptionalString(entry) : undefined;
if (trimmed) {
normalized.push(trimmed);
}
}
return normalized;
}
const single = typeof raw === "string" ? normalizeOptionalString(raw) : undefined;
return single ? [single] : [];
}
// Detects a declared media source field whose value is present but malformed
// (not a string for scalar sources; not a string or array for `mediaUrls`; or
// an array containing a non-string entry). A present malformed value expresses
// attachment intent the Feishu send path cannot represent, so it must fail
// visibly instead of being treated as absent and falling through to a
// text-only success branch that recreates the silent-drop bug of #112244.
// A blank string ("", " ") is a string and therefore not malformed.
function readFeishuParamPresence(
params: Record<string, unknown>,
key: string,
kind: "scalar" | "stringOrArray",
): "absent" | "present" | "malformed" {
const raw = readFeishuParam(params, key);
if (raw === undefined) {
return "absent";
}
if (kind === "stringOrArray") {
if (Array.isArray(raw)) {
return raw.some((entry) => typeof entry !== "string") ? "malformed" : "present";
}
return typeof raw === "string" ? "present" : "malformed";
}
return typeof raw === "string" ? "present" : "malformed";
}
// Detects an unsupported attachment-intent key (`file`/`buffer`/`base64`) whose
// value is present. A non-blank string is the original unsupported-intent
// signal; a present NON-string value (e.g. `file: {}`, `buffer: 42`) is a
// malformed intent the resolver cannot promote either. The non-string case used
// to be silently skipped — `readFeishuStringParam` returns undefined for
// objects, so `Boolean(...)` was false, leaving no candidate and falling through
// to a text-only `ok:true`, recreating the silent-drop this PR removes (issue
// #112244, ClawSweeper P1). A blank string ("", " ") stays ignored, preserving
// the blank-alias ignore behavior.
function hasFeishuUnsupportedIntentValue(params: Record<string, unknown>, key: string): boolean {
const raw = readFeishuParam(params, key);
if (raw === undefined) {
return false;
}
if (typeof raw === "string") {
return Boolean(normalizeOptionalString(raw));
}
return true;
}
type FeishuSendAttachmentMedia = {
mediaUrl: string | undefined;
/** Collected non-blank media source values across all aliases. */
mediaUrls: string[];
/** A buffer/base64 payload (top-level or nested in attachments[]) that the
* Feishu send path cannot represent; rejected instead of silent text fallback. */
hasUnsupportedAttachmentPayload: boolean;
/** An attachment-intent key (e.g. `file`) the Feishu send path does not map
* to a media source string; a non-blank value is rejected instead of silent
* text fallback (issue #112244). */
hasUnsupportedAttachmentIntent: boolean;
/** A declared media source field whose value is present but malformed (e.g.
* `media: {}` or `mediaUrls: [{}]`); rejected instead of being treated as
* absent and falling through to a text-only success branch (issue #112244). */
hasMalformedAttachmentIntent: boolean;
};
function collectFeishuSendAttachmentMedia(
params: Record<string, unknown>,
): FeishuSendAttachmentMedia {
const candidates: Array<string | undefined> = FEISHU_SEND_MEDIA_SOURCE_PARAM_KEYS.map((key) =>
readFeishuStringParam(params, key),
);
candidates.push(...readFeishuStringArrayParam(params, "mediaUrls"));
// A blank buffer/base64 ("", " ") is not an unsupported payload intent and
// must not trigger rejection; only a non-blank value counts. A present
// non-string value (e.g. `buffer: {}`) is a malformed payload intent that
// must also be rejected instead of silently skipped (issue #112244, ClawSweeper P1).
let hasUnsupportedAttachmentPayload =
hasFeishuUnsupportedIntentValue(params, "buffer") ||
hasFeishuUnsupportedIntentValue(params, "base64");
// A non-blank `file` is the linked issue's reproduction input; it is not a
// media source the Feishu send path maps, so it is treated as an unsupported
// attachment intent rather than dropped on the text-only success branch. A
// present non-string value (e.g. `file: {}`) is the same class of malformed
// intent and is rejected too (issue #112244, ClawSweeper P1).
let hasUnsupportedAttachmentIntent = hasFeishuUnsupportedIntentValue(params, "file");
// A declared media source whose value is present but malformed (non-string
// for scalar sources; non-string/non-array or array-with-non-string-entry for
// `mediaUrls`) is an attachment-intent signal the Feishu send path cannot
// satisfy, so it is rejected rather than silently treated as absent and
// dropped on the text-only success branch (issue #112244). A blank string is
// a string and therefore not malformed, preserving the blank-alias ignore.
let hasMalformedAttachmentIntent = false;
for (const key of FEISHU_SEND_MEDIA_SOURCE_PARAM_KEYS) {
if (readFeishuParamPresence(params, key, "scalar") === "malformed") {
hasMalformedAttachmentIntent = true;
}
}
if (readFeishuParamPresence(params, "mediaUrls", "stringOrArray") === "malformed") {
hasMalformedAttachmentIntent = true;
}
// A declared `attachments` field that is not an array (e.g. an object
// container like `{ media: "/tmp/a" }`) expresses attachment intent the
// Feishu send path cannot promote — the nested media source is not a
// top-level alias and the array resolver below never runs. Treating it as
// absent recreates the silent text-only `ok:true` success this PR removes,
// so it is rejected before the text branch (issue #112244, ClawSweeper P1).
if (params.attachments !== undefined && !Array.isArray(params.attachments)) {
hasMalformedAttachmentIntent = true;
}
if (Array.isArray(params.attachments)) {
for (const attachment of params.attachments) {
// A non-record array entry (e.g. `null`, a number, or a string) is a
// declared attachment intent the resolver cannot promote to a media
// source; skipping it silently would leave no candidate and fall through
// to a text-only `ok:true` — the exact drop this PR removes. Reject it
// before the text branch instead (issue #112244, ClawSweeper P1).
if (!isRecord(attachment)) {
hasMalformedAttachmentIntent = true;
continue;
}
for (const key of FEISHU_ATTACHMENT_MEDIA_SOURCE_PARAM_KEYS) {
candidates.push(readFeishuStringParam(attachment, key));
}
// Collect valid nested `mediaUrls` entries too, mirroring the top-level
// handling: without this, `{ attachments: [{ mediaUrls: ["/tmp/a.png"] }] }`
// would pass the malformed check but produce no candidate and silently
// fall through to a text-only `ok:true` — the exact drop this PR removes
// (issue #112244, ClawSweeper P1).
candidates.push(...readFeishuStringArrayParam(attachment, "mediaUrls"));
// Each structured attachment is scanned for unsupported payloads too, so
// `attachments: [{ buffer: ... }]` fails visibly instead of producing no
// media candidate and silently falling through to a text-only send. A
// present non-string value (e.g. `attachments: [{ file: {} }]`) is a
// malformed intent rejected the same way (issue #112244, ClawSweeper P1).
hasUnsupportedAttachmentPayload ||=
hasFeishuUnsupportedIntentValue(attachment, "buffer") ||
hasFeishuUnsupportedIntentValue(attachment, "base64");
hasUnsupportedAttachmentIntent ||= hasFeishuUnsupportedIntentValue(attachment, "file");
for (const key of FEISHU_ATTACHMENT_MEDIA_SOURCE_PARAM_KEYS) {
if (readFeishuParamPresence(attachment, key, "scalar") === "malformed") {
hasMalformedAttachmentIntent = true;
}
}
if (readFeishuParamPresence(attachment, "mediaUrls", "stringOrArray") === "malformed") {
hasMalformedAttachmentIntent = true;
}
}
}
const seen = new Set<string>();
const mediaUrls: string[] = [];
for (const candidate of candidates) {
if (!candidate || seen.has(candidate)) {
continue;
}
seen.add(candidate);
mediaUrls.push(candidate);
}
return {
mediaUrl: mediaUrls[0],
mediaUrls,
hasUnsupportedAttachmentPayload,
hasUnsupportedAttachmentIntent,
hasMalformedAttachmentIntent,
};
}
// Feishu delivers one media item per message. A send declaring more than one
// attachment is rejected rather than silently delivering only the first, and
// buffer/base64 payloads (top-level or nested in attachments[]) are rejected
// because the Feishu send path loads media from a media source string
// (path/URL) via the outbound adapter, not from an in-memory buffer. A
// non-blank `file` attachment-intent key is rejected the same way because the
// shared action contract deliberately excludes `file` from supported media
// sources; a present malformed media source value (e.g. `media: {}` or
// `mediaUrls: [{}]`) is rejected because the resolver cannot promote it to a
// mediaUrl and treating it as absent would recreate the silent text-only
// success branch. Failing visibly here matches the issue's expected behavior
// instead of returning ok:true on a text-only send. Mirrors the Mattermost
// send-attachment resolver shape.
function resolveFeishuSendAttachmentMedia(params: Record<string, unknown>): string | undefined {
const attachmentMedia = collectFeishuSendAttachmentMedia(params);
if (attachmentMedia.hasUnsupportedAttachmentPayload) {
throw new Error(
"Feishu send supports media attachments through media, mediaUrl, path, filePath, fileUrl, image, mediaUrls, or attachments[] with one of those fields; buffer/base64 payloads are not supported.",
);
}
if (attachmentMedia.hasMalformedAttachmentIntent) {
throw new Error(
"Feishu send supports media attachments through media, mediaUrl, path, filePath, fileUrl, image, mediaUrls, or attachments[] with one of those fields; a present malformed media source value is not supported — use a string path/URL (or a string array for mediaUrls) instead.",
);
}
if (attachmentMedia.hasUnsupportedAttachmentIntent) {
throw new Error(
"Feishu send supports media attachments through media, mediaUrl, path, filePath, fileUrl, image, mediaUrls, or attachments[] with one of those fields; the `file` attachment-intent parameter is not supported — use one of the supported media sources instead.",
);
}
if (attachmentMedia.mediaUrls.length > 1) {
throw new Error("Feishu send supports a single media attachment.");
}
return attachmentMedia.mediaUrl;
}
function readBooleanParam(params: Record<string, unknown>, keys: string[]): boolean | undefined {
for (const key of keys) {
const value = params[key];
@@ -1086,7 +1360,14 @@ export const feishuPlugin: ChannelPlugin<ResolvedFeishuAccount, FeishuProbeResul
const presentation =
normalizeMessagePresentation(ctx.params.presentation) ??
(interactive ? legacyInteractiveReplyToPresentation(interactive) : undefined);
const mediaUrl = readFeishuMediaParam(ctx.params);
// send promotes every attachment alias (path/filePath/mediaUrl/fileUrl/
// image/mediaUrls/attachments[]) to a canonical mediaUrl and rejects
// unsupported buffer/base64 payloads or multiple attachments instead
// of silently dropping them on the text-only success branch (#112244).
const mediaUrl =
ctx.action === "send"
? resolveFeishuSendAttachmentMedia(ctx.params)
: readFeishuMediaParam(ctx.params);
const audioAsVoice = readBooleanParam(ctx.params, ["asVoice", "audioAsVoice"]);
if (textCard && !presentation) {
assertFeishuCardWithinEnvelope(textCard, "Feishu native card");
@@ -1116,7 +1397,13 @@ export const feishuPlugin: ChannelPlugin<ResolvedFeishuAccount, FeishuProbeResul
if (mediaUrl && !maybeSendMedia) {
throw new Error("Feishu media sending is not available.");
}
const sendMedia = maybeSendMedia;
// The Feishu sendMedia implementation accepts an optional
// `propagateMediaUploadFailure` flag the shared adapter contract
// does not; `feishuOutbound` keeps the shared `ChannelOutboundAdapter`
// shape (optional `sendMedia`), so the direct send action narrows it
// to `FeishuOutboundSendMedia` here to request a controlled
// upload-failure outcome without a type assert.
const sendMedia: FeishuOutboundSendMedia | undefined = maybeSendMedia;
let result;
if (presentationFellBack && presentation) {
const sendPayload = runtime.feishuOutbound.sendPayload;
@@ -1135,6 +1422,20 @@ export const feishuPlugin: ChannelPlugin<ResolvedFeishuAccount, FeishuProbeResul
presentation,
...(mediaUrl ? { mediaUrl } : {}),
...(audioAsVoice === undefined ? {} : { audioAsVoice }),
// The direct `send` action must keep an attachment failure
// visible on this path too: `sendPayload`'s shared signature
// cannot carry `propagateMediaUploadFailure`, so stamp the
// marker here and let `sendFeishuFallbackPayload` re-throw the
// upload failure instead of returning a fallback-text `ok:true`
// receipt (issue #112244, ClawSweeper P1). Scoped to `send`
// only — thread-reply keeps its existing fallback behavior.
...(ctx.action === "send"
? {
channelData: {
feishu: { [FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER]: true },
},
}
: {}),
},
accountId: ctx.accountId ?? undefined,
...(ctx.mediaAccess ? { mediaAccess: ctx.mediaAccess } : {}),
@@ -1173,6 +1474,13 @@ export const feishuPlugin: ChannelPlugin<ResolvedFeishuAccount, FeishuProbeResul
? { threadId: replyToMessageId }
: { replyToId: replyToMessageId }),
...(audioAsVoice === true ? { audioAsVoice: true } : {}),
// The direct send action must not report `ok:true` when the
// requested attachment cannot be delivered; propagate the
// upload failure so the agent sees a visible error instead of
// a text-only fallback receipt (issue #112244). Scoped to
// `send` only — thread-reply keeps its existing fallback-text
// behavior for compatibility.
...(ctx.action === "send" ? { propagateMediaUploadFailure: true } : {}),
});
} else {
result = await runtime.sendMessageFeishu({
extensions/feishu/src/outbound.test.ts
+233 -2
View File
@@ -2,7 +2,10 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { createChannelPartialDeliveryError } from "openclaw/plugin-sdk/channel-inbound";
import {
createChannelPartialDeliveryError,
isChannelPartialDeliveryError,
} from "openclaw/plugin-sdk/channel-inbound";
import { verifyChannelMessageAdapterCapabilityProofs } from "openclaw/plugin-sdk/channel-outbound";
import {
adaptMessagePresentationForChannel,
@@ -119,7 +122,7 @@ vi.mock("./comment-reaction.js", () => ({
import { createFeishuCardInteractionEnvelope } from "./card-interaction.js";
import { feishuPlugin } from "./channel.js";
import { buildFeishuPostMessageContent } from "./markdown.js";
import { feishuOutbound } from "./outbound.js";
import { FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER, feishuOutbound } from "./outbound.js";
import { createFeishuSendReceipt } from "./send-result.js";
async function raceWithNextMacrotask<T>(promise: Promise<T>): Promise<T | "pending"> {
@@ -3142,6 +3145,234 @@ describe("feishuOutbound.sendMedia replyToId forwarding", () => {
expect(sendMessageCall()?.replyToMessageId).toBe("om_reply_target");
});
// Regression for #112244 (second-review P1): when the direct `send` action
// requests `propagateMediaUploadFailure`, a media-upload failure must re-throw
// to the caller instead of being converted to a fallback text success —
// otherwise the agent receives an `ok:true` receipt for a message whose
// attachment never arrived. No fallback "Media upload failed" text is emitted.
// When the caption was already delivered, the re-thrown error preserves that
// caption's receipt as the existing partial-delivery outcome (seventeenth-review
// P1) so the caller knows the text is visible and does not retry it.
it("propagates a media-upload failure instead of falling back to text when requested", async () => {
sendMessageFeishuMock.mockResolvedValueOnce({
messageId: "caption_msg",
chatId: "chat_1",
});
sendMediaFeishuMock.mockRejectedValueOnce(new Error("upload failed"));
await expect(
feishuOutbound.sendMedia?.({
cfg: emptyConfig,
to: "chat_1",
text: "see attachment",
mediaUrl: "https://example.com/file.png",
accountId: "main",
propagateMediaUploadFailure: true,
} as never),
).rejects.toThrow("upload failed");
expect(sendMediaFeishuMock).toHaveBeenCalledOnce();
// No fallback "Media upload failed" text is emitted on top of any caption.
expect(sendMessageCall()?.text).not.toContain("Media upload failed");
});
// Regression for #112244 (seventeenth-review P1): when the caption was already
// delivered before the media upload failed, the propagated error must preserve
// the caption's receipt as the repository's existing partial-delivery outcome
// — otherwise the caller treats it as a wholly failed send, retries, and
// duplicates the already-visible caption. Pre-fix the catch block threw a plain
// Error with no receipt; post-fix it throws a ChannelPartialDeliveryError
// carrying the caption's messageId.
it("preserves a delivered caption as partial delivery when media upload fails", async () => {
sendMessageFeishuMock.mockResolvedValueOnce({
messageId: "caption_msg",
chatId: "chat_1",
});
sendMediaFeishuMock.mockRejectedValueOnce(new Error("upload failed"));
let caught: unknown;
try {
await feishuOutbound.sendMedia?.({
cfg: emptyConfig,
to: "chat_1",
text: "see attachment",
mediaUrl: "https://example.com/file.png",
accountId: "main",
propagateMediaUploadFailure: true,
} as never);
} catch (err) {
caught = err;
}
expect(isChannelPartialDeliveryError(caught)).toBe(true);
const partial = caught as ReturnType<typeof createChannelPartialDeliveryError>;
expect(partial.deliveryResult.visibleReplySent).toBe(true);
expect(partial.deliveryResult.messageIds).toEqual(["caption_msg"]);
// The caption was delivered exactly once; no retry/duplicate send occurred.
expect(sendMessageFeishuMock).toHaveBeenCalledOnce();
expect(sendMediaFeishuMock).toHaveBeenCalledOnce();
});
// Regression for #112244 (seventeenth-review P1, scope guard): a media-upload
// failure with NO delivered caption is a wholly failed send, so the propagated
// error stays a plain Error (no partial-delivery receipt to preserve).
it("propagates a plain error when media upload fails with no delivered caption", async () => {
sendMediaFeishuMock.mockRejectedValueOnce(new Error("upload failed"));
let caught: unknown;
try {
await feishuOutbound.sendMedia?.({
cfg: emptyConfig,
to: "chat_1",
mediaUrl: "https://example.com/file.png",
accountId: "main",
propagateMediaUploadFailure: true,
} as never);
} catch (err) {
caught = err;
}
expect(isChannelPartialDeliveryError(caught)).toBe(false);
expect(caught).toBeInstanceOf(Error);
expect(String(caught)).toContain(
"Feishu send could not deliver the requested media attachment",
);
expect(sendMessageFeishuMock).not.toHaveBeenCalled();
});
it("still falls back to text on upload failure when propagation is not requested", async () => {
sendMediaFeishuMock.mockRejectedValueOnce(new Error("upload failed"));
const result = await feishuOutbound.sendMedia?.({
cfg: emptyConfig,
to: "chat_1",
text: "see attachment",
// A private/local media URL cannot be resolved to a public reference, so
// the fallback renders the generic "Media upload failed" text.
mediaUrl: path.join(os.tmpdir(), "openclaw-feishu-fallback-not-requested.png"),
accountId: "main",
});
// Default behavior is unchanged: the caption is sent first, then the
// fallback text, and a success receipt is returned (other outbound callers
// rely on this). The fallback is the second sendMessage call.
expect(sendMessageFeishuMock).toHaveBeenCalledTimes(2);
expect(sendMessageCall(1)?.text).toContain("Media upload failed. Please try again.");
expectFeishuResult(result, "text_msg");
});
// Regression for #112244 (third-review P1): the direct `send` action routes
// an attachment through the presentation-fallback path (sendPayload →
// sendFeishuFallbackPayload → sendMedia) when a card falls back. That path
// cannot carry `propagateMediaUploadFailure` through the shared sendPayload
// signature, so the action stamps the marker on channelData.feishu and the
// fallback payload must honor it — re-throwing the upload failure instead of
// returning a fallback-text `ok:true` receipt.
it("propagates a media-upload failure through the presentation-fallback path when the marker is set", async () => {
sendMediaFeishuMock.mockRejectedValueOnce(new Error("upload failed"));
await expect(
feishuOutbound.sendPayload?.({
cfg: emptyConfig,
to: "chat_1",
text: "see attachment",
accountId: "main",
payload: {
text: "see attachment",
mediaUrl: "https://example.com/file.png",
channelData: {
feishu: { [FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER]: true },
},
},
}),
).rejects.toThrow("Feishu send could not deliver the requested media attachment");
expect(sendMediaFeishuMock).toHaveBeenCalledOnce();
// No fallback "Media upload failed" text is emitted on top of any caption.
expect(
sendMessageFeishuMock.mock.calls
.map(([args]) => (args as { text?: string })?.text ?? "")
.some((text) => text.includes("Media upload failed")),
).toBe(false);
});
it("still falls back to text on the presentation-fallback path when the marker is absent", async () => {
sendMediaFeishuMock.mockRejectedValueOnce(new Error("upload failed"));
const result = await feishuOutbound.sendPayload?.({
cfg: emptyConfig,
to: "chat_1",
text: "see attachment",
accountId: "main",
payload: {
text: "see attachment",
// A private/local media URL cannot be resolved to a public reference,
// so the fallback renders the generic "Media upload failed" text.
mediaUrl: path.join(os.tmpdir(), "openclaw-feishu-fallback-no-marker.png"),
},
});
// Default behavior is unchanged: the caption is sent first, then the
// fallback text, and a success receipt is returned.
expect(sendMessageFeishuMock).toHaveBeenCalledTimes(2);
expect(sendMessageCall(1)?.text).toContain("Media upload failed. Please try again.");
expectFeishuResult(result, "text_msg");
});
// Regression for #112244 (fourteenth-review P1): the direct `send` action
// stamps the propagation marker on channelData.feishu before routing an
// attachment through sendPayload. The document-comment sendPayload branch
// preserves the marker (so the fallback helper requests propagation), but a
// comment target must NOT throw on the strength of that marker alone: the
// comment-target media-link fallback renders the media URL as a visible
// clickable link, which is established main behavior and NOT the silent
// text-only `ok:true` drop issue #112244 removes. Propagation is reserved for
// actual media-upload failures (the `sendMediaFeishu` catch), which a comment
// target never reaches because it never uploads. With the marker set, the
// comment target still renders the visible media-link fallback and returns a
// success receipt — the same outcome as the marker-absent case below.
it("renders the visible media-link fallback on a document-comment target even when the propagation marker is set", async () => {
const result = await feishuOutbound.sendPayload?.({
cfg: emptyConfig,
to: "comment:docx:doxcn123:7623358762119646411",
text: "see attachment",
accountId: "main",
payload: {
text: "see attachment",
mediaUrl: "https://example.com/pipeline.png",
channelData: {
feishu: { [FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER]: true },
},
},
});
// The media URL is delivered as a visible comment text link (not a silent
// text-only ok), and a success receipt is returned — comment targets keep
// their established media-link fallback regardless of the propagation flag.
expect(commentThreadParams()?.content).toBe("https://example.com/pipeline.png");
expectFeishuResult(result, "reply_msg");
// No media upload is attempted for a comment target.
expect(sendMediaFeishuMock).not.toHaveBeenCalled();
});
it("still degrades a comment attachment to text when the propagation marker is absent", async () => {
const result = await feishuOutbound.sendPayload?.({
cfg: emptyConfig,
to: "comment:docx:doxcn123:7623358762119646411",
text: "see attachment",
accountId: "main",
payload: {
text: "see attachment",
mediaUrl: "https://example.com/pipeline.png",
},
});
// Default comment behavior is unchanged: the media link is rendered as a
// fallback text comment and a success receipt is returned.
expect(commentThreadParams()?.content).toBe("https://example.com/pipeline.png");
expectFeishuResult(result, "reply_msg");
});
});
describe("feishuOutbound.sendMedia renderMode", () => {
extensions/feishu/src/outbound.ts
+131 -5
View File
@@ -1,6 +1,9 @@
// Feishu plugin module implements outbound behavior.
import path from "node:path";
import { isChannelPartialDeliveryError } from "openclaw/plugin-sdk/channel-inbound";
import {
isChannelPartialDeliveryError,
createChannelPartialDeliveryError,
} from "openclaw/plugin-sdk/channel-inbound";
import { createReplyToFanout } from "openclaw/plugin-sdk/channel-outbound";
import {
attachChannelToResult,
@@ -66,6 +69,15 @@ import {
const RENDERED_FEISHU_CARD = Symbol("openclaw.renderedFeishuCard");
const FEISHU_PRESENTATION_FALLBACK_MARKER = "__openclawPresentationFallback";
// Carries the direct-send upload-failure policy through the presentation
// fallback delivery path. The normal `sendMedia` branch sets
// `propagateMediaUploadFailure` directly; the presentation-fallback branch
// routes through `sendPayload` (whose shared signature cannot carry the flag),
// so the direct `send` action stamps this marker on `channelData.feishu` and
// `sendFeishuFallbackPayload` reads it before calling `sendMedia`. This keeps
// a direct-send attachment failure visible instead of degrading to a
// fallback-text `ok:true` receipt (issue #112244, ClawSweeper P1).
export const FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER = "__openclawPropagateMediaUploadFailure";
const FEISHU_TEXT_CHUNK_LIMIT = 4000;
function normalizePossibleLocalImagePath(text: string | undefined): string | null {
@@ -152,6 +164,23 @@ type FeishuOutboundPayload = Parameters<
type FeishuSendPayloadContext = Parameters<NonNullable<ChannelOutboundAdapter["sendPayload"]>>[0];
type FeishuSendTextContext = Parameters<NonNullable<ChannelOutboundAdapter["sendText"]>>[0];
// The Feishu sendMedia implementation accepts an optional flag the shared
// ChannelOutboundAdapter contract does not: when true, a media-upload failure
// is re-thrown to the caller instead of being converted to a fallback text
// success. The direct `send` action sets this so an agent that requested an
// attachment receives a visible failure when it cannot be delivered, rather
// than an `ok:true` receipt for a text-only fallback (issue #112244).
//
// The return type mirrors the shared contract exactly — `ReturnType<...>` is
// already `Promise<OutboundDeliveryResult>`, so wrapping it in another
// `Promise` would produce `Promise<Promise<...>>` and break the `async`
// implementation's single-Promise return (ClawSweeper P1).
export type FeishuOutboundSendMedia = (
params: Parameters<NonNullable<ChannelOutboundAdapter["sendMedia"]>>[0] & {
propagateMediaUploadFailure?: boolean;
},
) => ReturnType<NonNullable<ChannelOutboundAdapter["sendMedia"]>>;
function toFeishuOutboundResult<T extends { chatId: string }>(result: T) {
const { chatId, ...delivery } = result;
return { ...delivery, target: { kind: "chat" as const, id: chatId } };
@@ -190,6 +219,33 @@ function consumeFeishuPresentationFallbackMarker(payload: FeishuOutboundPayload)
};
}
// Reads (without consuming) the direct-send upload-failure policy stamped on
// the payload by the presentation-fallback branch. Unlike the presentation
// fallback marker this is not consumed: a fallback payload may fan out
// multiple `sendMedia` calls and each must honor the policy.
function readFeishuPropagateMediaUploadFailure(payload: FeishuOutboundPayload): boolean {
const feishuData = isRecord(payload.channelData?.feishu) ? payload.channelData.feishu : undefined;
return feishuData?.[FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER] === true;
}
// Builds a `channelData.feishu` that carries only the direct-send upload-failure
// policy marker. The document-comment branch strips native card / interactive
// channelData before fallback delivery (those cannot render in comments), but
// it must not drop the propagation marker the direct `send` action stamped —
// otherwise a media-upload failure on a comment target degrades to a
// fallback-text `ok:true` receipt again (issue #112244, ClawSweeper P1).
function buildFeishuPropagationOnlyChannelData(
payload: FeishuOutboundPayload,
): { feishu: Record<string, unknown> } | undefined {
const feishuData = isRecord(payload.channelData?.feishu) ? payload.channelData.feishu : undefined;
if (feishuData?.[FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER] !== true) {
return undefined;
}
return {
feishu: { [FEISHU_PROPAGATE_MEDIA_UPLOAD_FAILURE_MARKER]: true },
};
}
function buildFeishuPayloadCard(params: {
payload: Parameters<NonNullable<ChannelOutboundAdapter["sendPayload"]>>[0]["payload"];
text?: string;
@@ -484,12 +540,21 @@ async function sendFeishuFallbackPayload(params: {
payload: FeishuOutboundPayload;
separateMediaAndText?: boolean;
}) {
// The direct `send` action stamps this marker when it routes an attachment
// through the presentation-fallback path; honor it so an upload failure is
// re-thrown instead of degrading to a fallback-text `ok:true` receipt
// (issue #112244, ClawSweeper P1). The shared `sendTextMediaPayload` helper
// cannot carry the flag, so when propagation is requested and there is media
// to deliver, force the explicit fan-out path that calls `sendMedia` with
// the flag set.
const propagateMediaUploadFailure = readFeishuPropagateMediaUploadFailure(params.payload);
const ctx = { ...params.ctx, payload: params.payload };
const mediaUrls = normalizeStringEntries(resolvePayloadMediaUrls(params.payload));
const text = params.payload.text ?? "";
const textChunks = text ? chunkFeishuMarkdown(text, FEISHU_TEXT_CHUNK_LIMIT) : [];
const shouldSeparate =
mediaUrls.length > 0 && (params.separateMediaAndText === true || textChunks.length > 1);
mediaUrls.length > 0 &&
(propagateMediaUploadFailure || params.separateMediaAndText === true || textChunks.length > 1);
if (!shouldSeparate) {
return await sendTextMediaPayload({
channel: "feishu",
@@ -507,7 +572,11 @@ async function sendFeishuFallbackPayload(params: {
replyToIdSource: ctx.replyToIdSource,
replyToMode: ctx.replyToMode,
});
const sendMedia = feishuOutbound.sendMedia;
// Narrow the optional shared `sendMedia` to the Feishu-specific contract so
// the `propagateMediaUploadFailure` flag can be carried on direct-send
// fallback delivery (the shared `ChannelOutboundAdapter["sendMedia"]`
// signature does not declare it).
const sendMedia: FeishuOutboundSendMedia | undefined = feishuOutbound.sendMedia;
const sendText = feishuOutbound.sendText;
if (!sendMedia || !sendText) {
throw new Error("Feishu fallback delivery is not available.");
@@ -523,6 +592,7 @@ async function sendFeishuFallbackPayload(params: {
mediaUrl,
replyToId: nextReplyToId(),
audioAsVoice: params.payload.audioAsVoice ?? ctx.audioAsVoice,
...(propagateMediaUploadFailure ? { propagateMediaUploadFailure: true } : {}),
});
}
for (const chunk of textChunks) {
@@ -589,6 +659,14 @@ async function sendFeishuTtsSupplementPayload(params: {
return lastResult ?? { channel: "feishu", messageId: "" };
}
// `feishuOutbound` keeps the shared `ChannelOutboundAdapter` shape (whose
// `sendMedia` is optional) so the object literal — which spreads
// `createAttachedChannelResultAdapter` (returning `sendMedia?: ... | undefined`)
// — type-checks without a `sendMedia: ... | undefined` mismatch. Callers that
// need the Feishu-specific `propagateMediaUploadFailure` flag narrow the
// optional `sendMedia` to `FeishuOutboundSendMedia` at the use site
// (channel.ts direct-send branch, sendFeishuFallbackPayload) instead of
// forcing a required property here (ClawSweeper P1).
export const feishuOutbound: ChannelOutboundAdapter = {
deliveryMode: "direct",
chunker: chunkFeishuMarkdown,
@@ -654,7 +732,12 @@ export const feishuOutbound: ChannelOutboundAdapter = {
text,
interactive: undefined,
presentation: undefined,
channelData: undefined,
// Strip native card / interactive channelData (comments cannot render
// them) but preserve the direct-send upload-failure propagation marker
// so a media-upload failure on a comment target stays visible instead
// of degrading to a fallback-text `ok:true` receipt (issue #112244,
// ClawSweeper P1).
channelData: buildFeishuPropagationOnlyChannelData(payload),
};
return await sendFeishuFallbackPayload({
ctx,
@@ -930,6 +1013,14 @@ export const feishuOutbound: ChannelOutboundAdapter = {
replyToMode,
threadId,
onDeliveryResult,
propagateMediaUploadFailure = false,
}: Parameters<NonNullable<ChannelOutboundAdapter["sendMedia"]>>[0] & {
/** When true, a media-upload failure is re-thrown to the caller instead of
* being converted to a fallback text success. The direct `send` action
* sets this so an agent that requested an attachment receives a visible
* failure when the attachment cannot be delivered, rather than an `ok:true`
* receipt for a text-only fallback (issue #112244). */
propagateMediaUploadFailure?: boolean;
}) => {
const { normalizedReplyToId } = resolveFeishuReplyMode({
replyToId,
@@ -949,6 +1040,18 @@ export const feishuOutbound: ChannelOutboundAdapter = {
};
const deliveryOptions = { replyToIdSource, replyToMode, onDeliveryResult };
if (parseFeishuCommentTarget(to)) {
// Feishu document comments cannot host a media attachment; the mediaUrl
// is rendered as a fallback text link instead. This visible-link
// fallback is the established comment-target behavior on main and is
// NOT the silent text-only `ok:true` drop issue #112244 removes — the
// recipient sees the media URL as a clickable link, so the attachment
// intent is visibly delivered even though the comment cannot render the
// media inline. The direct-send upload-failure propagation policy
// therefore does not apply here: it targets actual media-upload failures
// (the `sendMediaFeishu` catch below), and a comment target never
// reaches that upload path. Keep the fallback for both `send` and
// thread-reply so comment targets retain their visible media-link
// delivery (issue #112244, ClawSweeper fourteenth-review P1).
const commentText = mediaUrl?.trim()
? await buildFeishuMediaFallbackText({
text,
@@ -986,10 +1089,11 @@ export const feishuOutbound: ChannelOutboundAdapter = {
audioAsVoice,
});
let textSent = false;
let captionResult: { messageId: string; chatId: string } | undefined;
// Send text first if provided, except for Feishu native voice bubbles.
if (text?.trim() && !suppressTextForVoiceMedia) {
await sendOutboundText({
captionResult = await sendOutboundText({
cfg,
to,
text,
@@ -1019,6 +1123,28 @@ export const feishuOutbound: ChannelOutboundAdapter = {
// Accepted media is not an upload failure and must never trigger a second send.
throw err;
}
if (propagateMediaUploadFailure) {
// The direct `send` action requested a controlled failure when the
// attachment cannot be delivered, so the agent receives a visible
// error instead of an `ok:true` receipt for a text-only fallback
// (issue #112244). When the caption was already delivered, preserve
// its receipt as the existing partial-delivery outcome so the caller
// knows the text is visible and does not retry it (which would
// duplicate the caption); only a send with no delivered caption is a
// wholly failed send.
if (textSent && captionResult) {
throw createChannelPartialDeliveryError(err, {
messageIds: [captionResult.messageId],
visibleReplySent: true,
});
}
throw new Error(
`Feishu send could not deliver the requested media attachment: ${
err instanceof Error ? err.message : String(err)
}`,
{ cause: err },
);
}
console.error(`[feishu] sendMediaFeishu failed:`, err);
const fallbackText = await buildFeishuMediaFallbackText({
text: textSent ? undefined : text,
src/gateway/server-methods/send.test.ts
+65
View File
@@ -17,6 +17,7 @@ import { createDeferred } from "../../../test/helpers/promise.js";
import { createOperationalRunInstanceRef } from "../../agents/admitted-run-context.js";
import { jsonResult } from "../../agents/tools/common.js";
import type { ChannelPlugin } from "../../channels/plugins/types.public.js";
import { createChannelPartialDeliveryError } from "../../channels/turn/delivery-result.js";
import type { SessionTranscriptAppendResult } from "../../config/sessions/transcript.js";
import {
claimAgentRunDelegatedAuthority,
@@ -1492,6 +1493,9 @@ describe("gateway send mirroring", () => {
} else {
expect(error).not.toHaveProperty("details");
}
// A queued or ordinary delivery failure must not advertise retryability;
// only a partial-delivery receipt sets `retryable: false`.
expect(error?.retryable).toBeUndefined();
});
it("does not send after delegated authority closes during session preparation", async () => {
@@ -4263,6 +4267,67 @@ describe("gateway send mirroring", () => {
expect(mocks.completeRestartRecoveryTerminalDelivery).not.toHaveBeenCalled();
});
it("returns the caption receipt through message.action when dispatch fails with partial delivery", async () => {
// A caption sent before the media upload failed carries a partial-delivery
// receipt. The Gateway boundary must surface that receipt on the structured
// error and mark the result non-retryable, so the agent does not resend an
// already-visible caption.
mocks.dispatchChannelMessageAction.mockRejectedValueOnce(
createChannelPartialDeliveryError(new Error("upload failed"), {
messageIds: ["caption_msg"],
visibleReplySent: true,
}),
);
const sessionKey = "agent:main:telegram:direct:chat-partial";
const { respond } = await runMessageActionRequest({
channel: "telegram",
action: "send",
params: { to: "chat-partial", message: "caption text" },
sessionKey,
sessionId: "session-partial",
agentId: "main",
idempotencyKey: "idem-partial-delivery",
});
const response = firstRespondCall(respond);
expect(response[0]).toBe(false);
expect(response[2]?.code).toBe(ErrorCodes.UNAVAILABLE);
expect(response[2]?.retryable).toBe(false);
expect(response[2]?.details).toMatchObject({
partialDelivery: {
messageIds: ["caption_msg"],
visibleReplySent: true,
},
});
expect(JSON.stringify(response[2])).toContain("caption_msg");
});
it("does not mark a plain unavailable failure as retryable", async () => {
// Without a partial-delivery receipt, the failure carries no retryable
// signal and no partial-delivery details, matching the pre-change shape
// (Gateway clients treat only `retryable === true` as permission to replay,
// so omitting it keeps an indeterminate send non-retryable).
mocks.dispatchChannelMessageAction.mockRejectedValueOnce(new Error("upload failed"));
const sessionKey = "agent:main:telegram:direct:chat-plain";
const { respond } = await runMessageActionRequest({
channel: "telegram",
action: "send",
params: { to: "chat-plain", message: "caption text" },
sessionKey,
sessionId: "session-plain",
agentId: "main",
idempotencyKey: "idem-plain-error",
});
const response = firstRespondCall(respond);
expect(response[0]).toBe(false);
expect(response[2]?.code).toBe(ErrorCodes.UNAVAILABLE);
expect(response[2]?.retryable).toBeUndefined();
expect(response[2]?.details).toBeUndefined();
});
it("passes reader-free agent-scoped media access to gateway attachment actions", async () => {
registerMessageActionPlugin({
action: "sendAttachment",
src/gateway/server-methods/send.ts
+21 -3
View File
@@ -21,6 +21,7 @@ import { dispatchChannelMessageAction } from "../../channels/plugins/message-act
import type { ChannelPlugin } from "../../channels/plugins/types.public.js";
import { resolveChannelThreadAddressing } from "../../channels/thread-addressing.js";
import type { InternalChannelThreadingToolContext } from "../../channels/threading-tool-context-internal.js";
import { isChannelPartialDeliveryError } from "../../channels/turn/delivery-result.js";
import { createOutboundSendDeps } from "../../cli/deps.js";
import {
getRuntimeConfigSnapshot,
@@ -840,12 +841,29 @@ function createGatewayInflightUnavailableFailure(params: {
channel: string;
err: unknown;
}): InflightResult {
// A channel partial-delivery error carries the receipt of the part that was
// already delivered (e.g. a caption sent before the media upload failed).
// Preserve it on the structured error and mark the result non-retryable so
// the agent does not resend an already-visible message; `String(err)` alone
// would drop the receipt and invite a duplicate delivery on retry.
const partialDelivery = isChannelPartialDeliveryError(params.err)
? params.err.deliveryResult
: undefined;
// A recovery-owned OutboundDeliveryError means the delivery was queued for
// retry by the recovery layer (not lost); surface that as a structured detail
// so the agent does not treat it as an ordinary retryable failure.
const queuedDelivery =
!partialDelivery &&
params.err instanceof OutboundDeliveryError &&
params.err.recoveryOwnedRetry === true;
const error = errorShape(
ErrorCodes.UNAVAILABLE,
String(params.err),
params.err instanceof OutboundDeliveryError && params.err.recoveryOwnedRetry === true
? { details: { code: GatewayErrorDetailCodes.OUTBOUND_DELIVERY_QUEUED } }
: undefined,
partialDelivery
? { details: { partialDelivery }, retryable: false }
: queuedDelivery
? { details: { code: GatewayErrorDetailCodes.OUTBOUND_DELIVERY_QUEUED } }
: undefined,
);
return createGatewayInflightResult({
...params,