feat(feishu): implement sendPayload for interactive card support#47891
feat(feishu): implement sendPayload for interactive card support#47891Horacehxw wants to merge 4 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR adds a Issues found:
Confidence Score: 3/5
|
| } catch (err) { | ||
| console.error(`[feishu] sendPayload media failed:`, err); | ||
| const fallbackText = text?.trim() ? `${text}\n\n📎 ${mediaUrl}` : `📎 ${mediaUrl}`; | ||
| const result = await sendOutboundText({ | ||
| cfg, | ||
| to, | ||
| text: fallbackText, | ||
| accountId: accountId ?? undefined, | ||
| replyToMessageId, | ||
| }); | ||
| return { channel: "feishu", ...result }; |
There was a problem hiding this comment.
Text sent twice when media upload fails
When text is present and sendMediaFeishu throws, the text has already been dispatched at line 107 via sendOutboundText. The catch block then builds fallbackText = \${text}\n\n📎 ${mediaUrl}`and callssendOutboundText` again, so the user receives the original text message followed immediately by a duplicate that also contains the media link.
The existing sendMedia method handles this identically-shaped path correctly — its fallback only sends 📎 ${mediaUrl} without re-including the text (line 217). sendPayload should match that behaviour:
| } catch (err) { | |
| console.error(`[feishu] sendPayload media failed:`, err); | |
| const fallbackText = text?.trim() ? `${text}\n\n📎 ${mediaUrl}` : `📎 ${mediaUrl}`; | |
| const result = await sendOutboundText({ | |
| cfg, | |
| to, | |
| text: fallbackText, | |
| accountId: accountId ?? undefined, | |
| replyToMessageId, | |
| }); | |
| return { channel: "feishu", ...result }; | |
| } catch (err) { | |
| console.error(`[feishu] sendPayload media failed:`, err); | |
| const fallbackText = `📎 ${mediaUrl}`; | |
| const result = await sendOutboundText({ | |
| cfg, | |
| to, | |
| text: fallbackText, | |
| accountId: accountId ?? undefined, | |
| replyToMessageId, | |
| }); | |
| return { channel: "feishu", ...result }; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/outbound.ts
Line: 124-134
Comment:
**Text sent twice when media upload fails**
When `text` is present and `sendMediaFeishu` throws, the text has already been dispatched at line 107 via `sendOutboundText`. The catch block then builds `fallbackText = \`${text}\n\n📎 ${mediaUrl}\`` and calls `sendOutboundText` **again**, so the user receives the original text message followed immediately by a duplicate that also contains the media link.
The existing `sendMedia` method handles this identically-shaped path correctly — its fallback only sends `📎 ${mediaUrl}` without re-including the text (line 217). `sendPayload` should match that behaviour:
```suggestion
} catch (err) {
console.error(`[feishu] sendPayload media failed:`, err);
const fallbackText = `📎 ${mediaUrl}`;
const result = await sendOutboundText({
cfg,
to,
text: fallbackText,
accountId: accountId ?? undefined,
replyToMessageId,
});
return { channel: "feishu", ...result };
}
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in 9423ec9 — sendPayload now short-circuits when there's no card, text, or media to send, avoiding empty API calls. The double-send bug (text before media + fallback text) was fixed by restructuring the fallback to only send a plain URL when media upload fails, without re-sending the text.
There was a problem hiding this comment.
Fixed in 35dee06 — the media-fail catch block no longer sends sendOutboundText again. The pre-failure text dispatch at line 107 was also moved: text is now sent only in the catch path, not before attempting media upload, so the text is never sent twice.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 766a97e438
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| // Fallback: send text + media via standard outbound paths. | ||
| const mediaUrl = payload.mediaUrl ?? payload.mediaUrls?.[0]; |
There was a problem hiding this comment.
Send every mediaUrls entry in sendPayload fallback
When channelData is present, outbound delivery now routes through feishuOutbound.sendPayload, but this fallback collapses attachments to payload.mediaUrl ?? payload.mediaUrls?.[0]. That silently drops the second and later entries from mediaUrls, so multi-attachment payloads with channel data regress from “all files sent” to “first file only”.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9423ec9 — sendPayload now iterates payload.mediaUrls (authoritative list) and only falls back to the legacy payload.mediaUrl scalar if mediaUrls is absent, matching the normalized payload contract.
There was a problem hiding this comment.
Fixed in 35dee06 — the fallback now iterates all entries in payload.mediaUrls (with each sent as a separate sendMediaFeishu call), not just payload.mediaUrl.
| const result = await sendMediaFeishu({ | ||
| cfg, | ||
| to, | ||
| mediaUrl, | ||
| accountId: accountId ?? undefined, | ||
| replyToMessageId, | ||
| }); |
There was a problem hiding this comment.
Pass mediaLocalRoots through sendPayload media path
The new sendPayload media fallback calls sendMediaFeishu without mediaLocalRoots, unlike sendMedia. sendMediaFeishu uses those roots to allow local-path reads, so channelData payloads that include local media paths now fail in this code path even though they succeed via the regular media sender.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eb26803 — sendPayload now passes mediaLocalRoots through to sendMediaFeishu in the media upload path, matching sendMedia's behavior and allowing local-image auto-upload.
There was a problem hiding this comment.
Fixed in 35dee06 — sendPayload now passes mediaLocalRoots from the runtime options through to each sendMediaFeishu call in the media-fallback path, matching how sendMedia handles it.
766a97e to
a561fae
Compare
|
All review feedback addressed in the latest push:
Tests: 18 passing in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a561faed34
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -81,6 +86,98 @@ export const feishuOutbound: ChannelOutboundAdapter = { | |||
| chunker: (text, limit) => getFeishuRuntime().channel.text.chunkMarkdownText(text, limit), | |||
| chunkerMode: "markdown", | |||
| textChunkLimit: 4000, | |||
| sendPayload: async ({ | |||
There was a problem hiding this comment.
Expose sendPayload from Feishu channel adapter
This commit adds feishuOutbound.sendPayload, but the exported Feishu plugin adapter still only forwards sendText/sendMedia (extensions/feishu/src/channel.ts:436-443), so outbound delivery never calls this new path. As a result, payloads that rely on channelData.feishu.card continue to bypass interactive-card sending in production even though unit tests on feishuOutbound now pass.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0cb8bf67 — added sendPayload forwarding to the channel.ts outbound object, matching the existing sendText/sendMedia pattern.
| to, | ||
| card: feishuData.card, | ||
| accountId: accountId ?? undefined, | ||
| replyToMessageId, | ||
| }); |
There was a problem hiding this comment.
Preserve thread mode for interactive card replies
In the interactive-card branch, sendCardFeishu gets replyToMessageId but never receives replyInThread, so when a delivery is scoped by threadId (without explicit replyToId) cards are sent as inline replies instead of topic-thread replies. sendCardFeishu supports replyInThread specifically for this behavior, and sendText already sets it for threaded card sends.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0cb8bf67 — sendPayload now passes replyInThread: threadId != null && !replyToId to sendCardFeishu, matching sendText's behavior. Added 2 tests covering both cases.
|
Additional fixes pushed in 0cb8bf67:
Tests: 20 passing in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cb8bf6739
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| // Text-only payload. | ||
| const result = await sendOutboundText({ |
There was a problem hiding this comment.
Chunk sendPayload fallback text before dispatch
This new sendPayload path sends fallback text with a single sendOutboundText call, so payloads routed through sendPayload (channelData/interactive deliveries) bypass Feishu chunking (chunker/textChunkLimit). Before this commit those non-card payloads were chunked by the normal sendText path in deliver, so long messages or stricter configured chunk limits can now fail instead of being split.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 396d195 — added sendOutboundTextChunked helper that splits text using chunkMarkdownText + resolveTextChunkLimit, applied to both the 'text before media' path and the 'text-only fallback' path in sendPayload.
There was a problem hiding this comment.
Fixed in 396d195 — added a sendOutboundTextChunked helper that uses resolveTextChunkLimit and chunkMarkdownText (matching core sendTextChunks behavior). Applied to both the 'text before media' path and the 'text-only fallback' path in sendPayload. A test covers the two-chunk split case.
0cb8bf6 to
d86311a
Compare
|
Rebased onto latest Changes from rebase:Upstream adaptation: Review feedback status:All P1/P2 review items are addressed in this commit:
All 20 tests pass. TypeScript, format, and lint checks green. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d86311a9e4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Text-only payload. | ||
| return attachChannelToResult( | ||
| "feishu", | ||
| await sendOutboundText({ | ||
| cfg, | ||
| to, | ||
| text: text || "", | ||
| accountId: accountId ?? undefined, | ||
| replyToMessageId, | ||
| }), |
There was a problem hiding this comment.
Preserve local-image auto-upload in sendPayload fallback
When a payload carries channelData but no channelData.feishu.card, deliverOutboundPayloads now routes it through sendPayload instead of falling through to sendText (src/infra/outbound/deliver.ts:676-683). This text-only fallback calls sendOutboundText directly, so it skips the local-image compatibility shim in sendText (extensions/feishu/src/outbound.ts:202-215) that turns an absolute image path into sendMediaFeishu. As a result, any rich payload whose text is a local image path will now leak /tmp/...png into chat instead of uploading the image.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eb26803 — sendPayload text-only path now runs the local-image auto-upload shim (matching sendText) before falling through to sendOutboundTextChunked.
There was a problem hiding this comment.
Fixed in eb26803 — added a local-image auto-upload shim to the sendPayload text-only fallback path, matching the shim already in sendText. Local paths are uploaded before text dispatch so they don't leak as raw file paths into chat.
d86311a to
cbd735b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbd735b408
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (payload.mediaUrl) { | ||
| mediaUrls.push(payload.mediaUrl); |
There was a problem hiding this comment.
Respect mediaUrls precedence over legacy mediaUrl
This fallback unconditionally pushes payload.mediaUrl before iterating payload.mediaUrls, which breaks the repo’s normalized payload contract where mediaUrls is authoritative and mediaUrl is only a legacy fallback. When both fields are present with different values, this path now sends an extra stale/duplicate attachment that the previous Feishu delivery path would have ignored. Build the list from mediaUrls first and only use mediaUrl when mediaUrls is empty.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9423ec9 — reversed the order: iterate payload.mediaUrls first, and only push payload.mediaUrl (legacy scalar) if mediaUrls is absent or empty. This matches the normalized payload contract.
There was a problem hiding this comment.
Fixed in 9423ec9 — mediaUrls is now used as the authoritative source. When both mediaUrls and the legacy mediaUrl are present, only mediaUrls entries are iterated. mediaUrl is only used as a single-item fallback when mediaUrls is absent, matching the normalized payload contract.
| await sendOutboundText({ | ||
| cfg, | ||
| to, | ||
| text: text || "", |
There was a problem hiding this comment.
Short-circuit empty sendPayload fallback content
deliverOutboundPayloads routes payloads with interactive or channelData into sendPayload; in this branch, if no Feishu card and no media are present, the code still calls sendOutboundText with text || "". That can turn interactive-only/channelData-only payloads into blank Feishu posts (or hard API failures), instead of a safe no-op result. Guard this fallback so it returns an empty delivery result when there is no text/media/card content to send.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9423ec9 — added an early return at the top of sendPayload when no Feishu card, media, or text is present, avoiding empty postFeishu API calls.
There was a problem hiding this comment.
Fixed in 9423ec9 — sendPayload now short-circuits and returns early when there is no text, no media, and no card to send, avoiding empty Feishu API calls.
cbd735b to
20a41a8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a41a8184
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| cfg, | ||
| to, | ||
| text: text || "", | ||
| accountId: accountId ?? undefined, | ||
| replyToMessageId, |
There was a problem hiding this comment.
Preserve identity headers in sendPayload text fallback
When deliverOutboundPayloads routes channelData/interactive payloads into sendPayload, this fallback sends text through sendOutboundText directly, which never uses identity. In Feishu renderMode: "card" (or auto-card detection), that skips the sendText structured-card path that adds the sender header (identity.name/emoji), so identity-attributed messages lose their header after this commit whenever they include channelData but no channelData.feishu.card.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eb26803 — the text fallback in sendPayload now calls sendOutboundText with the identity parameter, routing through sendStructuredCardFeishu which properly includes identity headers in the Feishu message card.
There was a problem hiding this comment.
Fixed in eb26803 — sendOutboundText was extended to accept replyInThread and identity options. The card-mode text fallback in sendPayload now passes both, routing through sendStructuredCardFeishu when an identity is set (matching sendText's behavior).
20a41a8 to
6ad8173
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ad8173437
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await sendOutboundText({ | ||
| cfg, | ||
| to, | ||
| text, | ||
| accountId: accountId ?? undefined, |
There was a problem hiding this comment.
Preserve thread replies for card-mode sendPayload text fallback
When sendPayload falls back to text (for payloads with channelData but no channelData.feishu.card), it calls sendOutboundText with only replyToMessageId. In card render mode (renderMode: "card" or auto-detected card text) this path sends via sendMarkdownCardFeishu without replyInThread, so threadId-scoped replies are posted as inline replies instead of topic-thread replies. Before this change, the same payloads went through sendText, which sets replyInThread: threadId != null && !replyToId for card sends.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eb26803 — sendPayload now extracts replyInThread at the top level and passes it to both the interactive-card path and the text-fallback path via sendOutboundText.
There was a problem hiding this comment.
Fixed in eb26803 — replyInThread is now extracted at the top of sendPayload and passed through to both the interactive-card path and the text-fallback path, so threaded card replies are preserved in both cases.
6ad8173 to
1722a33
Compare
1722a33 to
f6d31c5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6d31c550e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await sendOutboundText({ | ||
| cfg, | ||
| to, | ||
| text, | ||
| accountId: accountId ?? undefined, |
There was a problem hiding this comment.
Chunk sendPayload text before direct Feishu send
sendPayload sends text-only fallbacks with a single sendOutboundText call, but payloads with interactive or channelData are routed directly to sendPayload and bypass the core sendText chunking path. This means long interactive/channelData text now goes out unchunked and can exceed Feishu limits, causing delivery failures that previously would have been split into safe chunks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 396d195 — both the 'text before media' path and the 'text-only fallback' path now use sendOutboundTextChunked to split text at the configured chunk limit before calling sendOutboundText.
There was a problem hiding this comment.
Fixed in 396d195 — both the 'text before media' path and the 'text-only fallback' path now go through sendOutboundTextChunked, which splits long text using chunkMarkdownText respecting resolveTextChunkLimit. Total 26 passing tests in outbound.test.ts including a two-chunk split test.
There was a problem hiding this comment.
Fixed in 396d195 — added sendOutboundTextChunked helper that calls resolveTextChunkLimit and chunkMarkdownText (matching core sendTextChunks behavior). Applied to both the 'text before media' path and the 'text-only fallback' path in sendPayload. A test covers the two-chunk split case (26 passing tests in outbound.test.ts).
There was a problem hiding this comment.
Fixed — sendPayload now uses sendOutboundTextChunked (lines 248-259) which applies the same chunking logic as the core sendText path, preventing oversized messages from exceeding Feishu limits.
f6d31c5 to
c069267
Compare
Add sendPayload to the Feishu outbound adapter enabling interactive card delivery via channelData.feishu.card. Falls back to text+media paths for payloads without card data. - Wire sendPayload through channel.ts runtime delegate - Handle multi-attachment mediaUrls (iterate all, not just first) - Pass mediaLocalRoots through to sendMediaFeishu - Pass replyInThread to sendCardFeishu for threaded card replies - Avoid text duplication in media-fail fallback - Use attachChannelToResult for channel tagging (compatible with createAttachedChannelResultAdapter refactor on main)
- Prefer mediaUrls (authoritative) over legacy mediaUrl when both are present, matching the normalized payload contract - Short-circuit sendPayload when no text/media/card content to send, avoiding empty Feishu post API calls - Add 2 tests covering the above cases (total: 22 passing)
- Extend sendOutboundText to accept replyInThread and identity, routing card-mode text through sendStructuredCardFeishu (supports both header and replyInThread), matching what sendText already does - Extract replyInThread at the top of sendPayload and reuse for both interactive-card and text-fallback paths - Add local-image auto-upload shim to sendPayload text-only fallback, preventing local paths from leaking into chat (matches sendText shim) - Update sendMedia caption test to reflect sendStructuredCardFeishu usage - Add 3 tests: identity header in card-mode fallback, replyInThread in card-mode fallback, and local-image auto-upload in sendPayload fallback (total: 25 passing) Also remove unused normalizeGoogleApiBaseUrl imports in two unrelated files introduced by upstream refactor (pre-existing lint failures).
…ge limit The core delivery loop calls sendPayload directly, bypassing sendTextChunks, so long text in channelData/interactive payloads was sent unchunked and could exceed Feishu API limits. Add sendOutboundTextChunked helper that resolves the configured text limit via resolveTextChunkLimit and splits text using the existing chunkMarkdownText chunker, matching core sendTextChunks behaviour. Apply to both the "text before media" path and the "text-only fallback" path in sendPayload. Update the shared runtime mock in outbound.test.ts to expose resolveTextChunkLimit, and add a test covering the two-chunk split case. Total: 26 passing tests in outbound.test.ts.
c069267 to
396d195
Compare
Summary
sendPayloadonfeishuOutboundadapter to supportmsg_type: "interactive"(Feishu interactive cards)sendCardFeishu()fromsend.ts— no new send function neededCloses #47890
Motivation
Agents sending rich daily reports (column layouts + embedded charts + markdown analysis) currently must bypass OpenClaw's native Feishu channel and call the Feishu API directly. This breaks conversation context and requires credential injection into sandbox containers.
With
sendPayload, agents can send interactive cards through the native outbound adapter, keeping messages in the conversation context and credentials managed by OpenClaw.Changes
extensions/feishu/src/outbound.ts(+65 lines)sendPayloadmethod tofeishuOutboundpayload.channelData?.feishu?.cardfor interactive card contentsendCardFeishu()formsg_type: "interactive"sendOutboundText()+sendMediaFeishu()for non-card payloadsTest plan
tsgo --noEmitpasses with zero errorssendPayloadsendText/sendMediapaths are unaffected🤖 Generated with Claude Code