fix(feishu): support v2 card format, streaming cards, and interactive message type parsing#39339
fix(feishu): support v2 card format, streaming cards, and interactive message type parsing#39339guoqunabc wants to merge 2 commits intoopenclaw:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3bc4c3e6d
ℹ️ 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".
| card.i18n_elements.zh_cn ?? | ||
| (Object.values(card.i18n_elements).find((v) => Array.isArray(v)) as unknown[] | undefined); |
There was a problem hiding this comment.
Skip empty zh_cn locale before falling back
The locale fallback for i18n_elements uses nullish coalescing, so if zh_cn is present but empty (or present with a non-array value), it is still selected and the parser never checks other locales. In that case cards that contain usable text in en_us (or another locale) are reduced to [Interactive Card] even though content exists. Guard zh_cn with an array/non-empty check before preferring it, then fall back to the first non-empty locale array.
Useful? React with 👍 / 👎.
xkonjin
left a comment
There was a problem hiding this comment.
Quick review pass:
- Main risk area here is edge cases on the new/changed execution path.
- I didn’t see targeted regression coverage in the diff; please add or point CI at a focused test for the changed path in bot.ts, send.ts.
- Before merge, I’d smoke-test the behavior touched by bot.ts, send.ts with malformed input / retry / rollback cases, since that’s where this class of change usually breaks.
Greptile SummaryThis PR fixes three distinct issues with Feishu interactive card message parsing: v2 ( The implementation is well-structured:
The only type-safety note: the Note: Existing test coverage only exercises the original v1 flat-elements format. The new card-format code paths (v2 Confidence Score: 4/5
Last reviewed commit: f3bc4c3 |
| if (card.i18n_elements && typeof card.i18n_elements === "object") { | ||
| elements = | ||
| card.i18n_elements.zh_cn ?? | ||
| (Object.values(card.i18n_elements).find((v) => Array.isArray(v)) as unknown[] | undefined); | ||
| } |
There was a problem hiding this comment.
The i18n_elements field is typed as Record<string, unknown[]>, which means TypeScript (without noUncheckedIndexedAccess) considers every locale key unconditionally defined. The ?? fallback works correctly at runtime, but the type annotation doesn't reflect that keys like zh_cn might be missing.
Consider using a more precise type to help the compiler understand the actual runtime behavior:
| if (card.i18n_elements && typeof card.i18n_elements === "object") { | |
| elements = | |
| card.i18n_elements.zh_cn ?? | |
| (Object.values(card.i18n_elements).find((v) => Array.isArray(v)) as unknown[] | undefined); | |
| } | |
| i18n_elements?: { zh_cn?: unknown[]; [locale: string]: unknown[] | undefined }; |
This is a non-breaking improvement that would strengthen type safety without affecting runtime behavior.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/send.ts
Line: 179-183
Comment:
The `i18n_elements` field is typed as `Record<string, unknown[]>`, which means TypeScript (without `noUncheckedIndexedAccess`) considers every locale key unconditionally defined. The `??` fallback works correctly at runtime, but the type annotation doesn't reflect that keys like `zh_cn` might be missing.
Consider using a more precise type to help the compiler understand the actual runtime behavior:
```suggestion
i18n_elements?: { zh_cn?: unknown[]; [locale: string]: unknown[] | undefined };
```
This is a non-breaking improvement that would strengthen type safety without affecting runtime behavior.
How can I resolve this? If you propose a fix, please make it concise.f3bc4c3 to
0536039
Compare
|
Addressed the review feedback with focused regression coverage and malformed-input edge cases. What changed:
Validation run in this branch:
Type-check note:
|
1 similar comment
|
Addressed the review feedback with focused regression coverage and malformed-input edge cases. What changed:
Validation run in this branch:
Type-check note:
|
|
Addressed the latest review feedback in follow-up commit 62a8ee0a3. Updates in this round:
Also clarifying an operational safety point: this PR is meant to go through the normal repo build/release flow, not to be copied into a live installed package by hand. 本轮已补上 reviewer 关心的 malformed input / edge-case 覆盖，并更新 PR 描述说明测试范围与安全边界。 |
bb7a5f9 to
2ba9454
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69fa88ce14
ℹ️ 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".
| (Array.isArray(zhCn) && zhCn.length > 0 ? zhCn : undefined) ?? | ||
| (Object.values(card.i18n_elements).find((v) => Array.isArray(v) && v.length > 0) as | ||
| | unknown[] |
There was a problem hiding this comment.
Fallback to other locales when zh_cn has no text
Locale selection currently treats any non-empty zh_cn array as final, even when those elements contain no extractable div/markdown text. In that case, cards with readable text in another locale (for example en_us) still degrade to [Interactive Card] because extraction never retries a secondary locale. This is a content-loss regression for i18n cards where the preferred locale is present but non-textual.
Useful? React with 👍 / 👎.
e5c8f31 to
fd3d27a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd3d27a149
ℹ️ 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".
| let elements: unknown[] | undefined = card.elements; | ||
| if (!Array.isArray(elements)) { | ||
| // Try body.elements (card kit v2 / schema 2.0) | ||
| elements = card.body?.elements; | ||
| } |
There was a problem hiding this comment.
Continue fallback when top-level elements is empty
The element-source resolution stops as soon as elements is an array, so elements: [] is treated as authoritative and prevents checking body.elements/i18n_elements. This drops real card text for mixed payloads that carry an empty compatibility elements field plus populated v2 content under body.elements, causing [Interactive Card] even though readable content exists.
Useful? React with 👍 / 👎.
| if (messageType === "interactive") { | ||
| return parseInteractiveCardContent(parsed); |
There was a problem hiding this comment.
Preserve raw content when interactive parsing fails
Direct interactive messages now always return parseInteractiveCardContent(parsed), which collapses unsupported card structures to [Interactive Card]; unlike the previous behavior, the original JSON payload is no longer available to the agent. For cards whose visible/semantic text is outside the currently handled tags, this is a regression in message fidelity and can hide actionable content.
Useful? React with 👍 / 👎.
fd3d27a to
12b2745
Compare
…ontent parsing - Enhance parseInteractiveCardContent to handle card kit v2 (body.elements), header titles, i18n_elements, template cards, column_set, and streaming card references - Extract helper extractTextsFromElements for recursive element traversal - Export parseInteractiveCardContent for reuse across modules - Add interactive message type handling in bot.ts parseMessageContent Previously, quoting a card message built with buildMarkdownCard (schema 2.0) would show '[Interactive Card]' because the parser only checked top-level elements. Streaming card references were also unhandled.
12b2745 to
c34946d
Compare
|
Hi @xkonjin, I've added the regression tests you requested — covering the changed |
|
Closing this PR — the official Feishu plugin now covers streaming cards and v2 card support, making this change unnecessary. Thanks for the reviews! |
Summary / 概要
Fix three Feishu interactive-card parsing gaps and strengthen regression coverage for malformed payloads.
修复飞书 interactive card 解析的三个缺口，并补强 malformed payload 的回归测试覆盖。
What this PR fixes / 修复内容
parseInteractiveCardContentnow handles schema 2.0 cards that store content underbody.elements, not only v1 cards with top-levelelements.{"type":"card","data":{...}}is parsed more usefully — extract inline text when available, otherwise fall back safely.interactivemessage as the primary message (not just quoted content),bot.tsnow parses it into readable text instead of returning raw JSON.Safety note / 安全说明
This PR is intended to be merged and consumed through the normal OpenClaw build/release flow.
Do not copy source files from a development branch into a live installed OpenClaw package.
Please avoid hand-patching
~/.npm-global/lib/node_modules/openclaw/...or any running production install with repo source files.本 PR 应通过正常的构建 / 发布流程进入版本。
不要把开发分支源码直接复制到正在运行的 OpenClaw 安装目录中。
请不要手工 patch
~/.npm-global/lib/node_modules/openclaw/...或任何线上运行中的安装包。Implementation details / 实现细节
extensions/feishu/src/send.tsextractTextsFromElementsfor recursive traversalelementsbody.elementsi18n_elementslocale fallback withzh_cnpreferencecolumn_set, form-like and collapsible structuresi18n_elementstyping for better type safetyextensions/feishu/src/bot.tsparseInteractiveCardContentfor directinteractivemessage parsingTests / 测试
Focused regression coverage now includes:
body.elementselementscard_idi18n_elementslocale preference and empty-array fallbackinteractivemessage parsing inbot.ts[Interactive Card]Ran:
pnpm vitest run extensions/feishu/src/send.test.ts extensions/feishu/src/bot.test.ts --config vitest.extensions.config.tspnpm format:check -- extensions/feishu/src/send.ts extensions/feishu/src/bot.ts extensions/feishu/src/send.test.ts extensions/feishu/src/bot.test.tsReviewer follow-up / 对 review 意见的回应
send.ts/bot.tsexecution pathsRelated / 相关