feat(cards): add shared adaptive card rendering for all channels#41565
feat(cards): add shared adaptive card rendering for all channels#41565VikrantSingh01 wants to merge 5 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
bot
added
channel: msteams
Greptile SummaryThis PR introduces a well-structured shared adaptive card rendering layer ( Issues found:
Confidence Score: 2/5
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/cards/strategies/telegram.ts
Outdated
| if (!label) { | ||
| continue; | ||
| } | ||
| if (action.type === "Action.OpenUrl") { | ||
| buttons.push({ text: label, url: str(action.url) }); | ||
| } else if (action.type === "Action.Submit") { | ||
| // Encode action data as callback_data (Telegram limit: 64 bytes) | ||
| const data = action.data != null ? JSON.stringify(action.data) : label; | ||
| buttons.push({ text: label, callback_data: data.slice(0, 64) }); |
There was a problem hiding this comment.
callback_data truncated by character count, not byte count
The comment correctly states the Telegram limit is 64 bytes, but .slice(0, 64) counts UTF-16 code units (characters), not bytes. A JSON payload containing multi-byte characters (e.g. Chinese, emoji) can exceed 64 bytes even after slicing to 64 characters, which will cause Telegram's Bot API to return a 400 error (Bad Request: callback_data is too long).
Use a TextEncoder (or a simple UTF-8 byte-length check) to enforce the byte limit:
| if (!label) { | |
| continue; | |
| } | |
| if (action.type === "Action.OpenUrl") { | |
| buttons.push({ text: label, url: str(action.url) }); | |
| } else if (action.type === "Action.Submit") { | |
| // Encode action data as callback_data (Telegram limit: 64 bytes) | |
| const data = action.data != null ? JSON.stringify(action.data) : label; | |
| buttons.push({ text: label, callback_data: data.slice(0, 64) }); | |
| } else if (action.type === "Action.Submit") { | |
| // Encode action data as callback_data (Telegram limit: 64 bytes, not chars) | |
| const raw = action.data != null ? JSON.stringify(action.data) : label; | |
| // Truncate by encoded byte length, not character count | |
| const encoder = new TextEncoder(); | |
| let truncated = raw; | |
| while (encoder.encode(truncated).byteLength > 64) { | |
| truncated = truncated.slice(0, -1); | |
| } | |
| buttons.push({ text: label, callback_data: truncated }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cards/strategies/telegram.ts
Line: 85-93
Comment:
**`callback_data` truncated by character count, not byte count**
The comment correctly states the Telegram limit is **64 bytes**, but `.slice(0, 64)` counts UTF-16 code units (characters), not bytes. A JSON payload containing multi-byte characters (e.g. Chinese, emoji) can exceed 64 bytes even after slicing to 64 characters, which will cause Telegram's Bot API to return a 400 error (`Bad Request: callback_data is too long`).
Use a `TextEncoder` (or a simple UTF-8 byte-length check) to enforce the byte limit:
```suggestion
} else if (action.type === "Action.Submit") {
// Encode action data as callback_data (Telegram limit: 64 bytes, not chars)
const raw = action.data != null ? JSON.stringify(action.data) : label;
// Truncate by encoded byte length, not character count
const encoder = new TextEncoder();
let truncated = raw;
while (encoder.encode(truncated).byteLength > 64) {
truncated = truncated.slice(0, -1);
}
buttons.push({ text: label, callback_data: truncated });
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in 226b991. Now truncates by UTF-8 byte length using TextEncoder instead of character count. The loop slices one character at a time until encoder.encode(truncated).byteLength <= 64.
|
All 4 issues from the Greptile review addressed in 226b991: 1. Empty URL guard (Logic — all 3 strategies) 2. Telegram 3. Slack raw marker leak (Logic) 4. Slack FactSet 10-field limit (Style) |
VikrantSingh01
force-pushed
the
feat/adaptive-cards-channel-rendering
branch
from
d7777ae to
09f3ace
Compare
…uncation, marker leak prevention - Skip Action.OpenUrl with empty URL in all 3 strategies (Discord, Slack, Telegram) to prevent 400 API errors - Telegram: truncate callback_data by UTF-8 byte length (not char count) to respect the 64-byte Telegram limit with multi-byte characters - Slack: split FactSets with >10 facts into multiple section blocks to respect Slack's 10-field-per-section API limit - All outbound adapters: strip card markers from text when rendering fails or produces empty output, preventing raw JSON marker leak to end users on Slack, Telegram, Discord, and Teams Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…y tests - Wrap strategy.render() in try/catch in all 4 outbound adapters to prevent message delivery crash on malformed card data - Discord: enforce API limits (title 256 chars, description 4096 chars, 25 fields max, field name/value 1024 chars) - Slack: truncate mrkdwn text to 3000 chars per section block - Add 12 strategy tests covering Telegram, Slack, Discord, and native strategies (empty URL guard, byte-limit truncation, FactSet splitting, Discord field caps, native pass-through) - Total test count: 22 (10 parser + 12 strategy) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
VikrantSingh01
force-pushed
the
feat/adaptive-cards-channel-rendering
branch
from
09f3ace to
c0cfc41
Compare
openclaw-barnacle
bot
added
channel: discord
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
BradGroux
left a comment
BradGroux
left a comment
There was a problem hiding this comment.
Review: Shared Channel Rendering Infrastructure (Core Engine)
This is the foundation PR that all platform renderers depend on. 1,843 lines across 13 files covering the parser, strategy pattern, and per-channel rendering (Discord embeds+buttons, Slack Block Kit, Telegram HTML+inline keyboard, Teams native pass-through). Includes 35 tests. The architecture is solid and the strategy pattern is the right call for channel isolation.
Blocker
1. Discord action rows can exceed the 5-button API limit
src/cards/strategies/discord.ts (and mirrored in extensions/discord/src/outbound-adapter.ts) builds a single action row containing all card actions with no cap:
function buildActionRow(actions: unknown[]): unknown {
const buttons: DiscordButton[] = [];
// ... pushes all actions into one row
return { type: 1, components: buttons };
}Discord enforces a hard limit of 5 buttons per action row. If a card has more than 5 actions (including combined top-level + ActionSet actions), Discord will reject the entire payload and the message send fails silently. This needs to chunk actions into multiple rows of 5 or truncate with a warning.
Suggestions
2. Marker stripping inconsistency leaks meta payload in extension adapters
The core parser (src/cards/parse.ts) strips all three marker types: adaptive-card, adaptive-card-data, and adaptive-card-meta. However, each extension adapter defines its own AC_MARKERS_RE regex that only strips adaptive-card and adaptive-card-data, omitting adaptive-card-meta:
// In extensions/discord/src/outbound-adapter.ts (and slack, telegram, msteams):
const AC_MARKERS_RE = /<!--adaptive-card-->...|<!--adaptive-card-data-->.../g;On fallback paths where the extension strips markers but does not use the core parser, raw <!--adaptive-card-meta--> JSON comments will leak into user-visible chat text. The extension regex should match all three markers, or better yet, import the regex from the core module to avoid drift.
3. Parser does not validate card.body is an array before returning
src/cards/parse.ts checks card.type === "AdaptiveCard" but never verifies Array.isArray(card.body):
if (card?.type !== "AdaptiveCard") {
return null;
}
// ... returns { card, fallbackText, ... } without body validationAll downstream strategies iterate over parsed.card.body. A malformed card with body: "string" or body: null will throw at render time unless every strategy wraps iteration in try/catch. Adding a guard here would be defensive and cheap.
Architecture Notes (not blocking)
- The strategy pattern with per-channel isolated renderers is clean. Each strategy wraps render in try/catch with text fallback, which is the right safety net.
- The
str()helper function is duplicated across multiple strategy files. Consider extracting to a shared utility, though this is a nit for a follow-up PR. - The marker protocol (
<!--adaptive-card-->JSON<!--/adaptive-card-->+ data + meta) is well-designed and extensible.
Summary
Strong foundation. Fix the Discord 5-button limit (this will cause real failures in production) and align the marker regex across core and extensions to prevent meta marker leakage. The body validation guard is cheap insurance against malformed payloads.
2 participants
Summary
Adds a shared card rendering layer that enables any channel to consume Adaptive Cards natively. Works with any plugin that emits
<!--adaptive-card-->markers in tool result text, including the standalone@vikrantsingh01/openclaw-adaptive-cardsplugin (v4.0.0).<!--adaptive-card-->markers embedded in tool result textNote: After rebase on main, the outbound adapters now live in
extensions/*/src/outbound-adapter.ts(previouslysrc/channels/plugins/outbound/). Card rendering logic has been applied to the new extension locations with inline parsing (no cross-workspace imports).Channel Rendering Matrix
Architecture
Each strategy is a pure function (ParsedAdaptiveCard -> CardRenderResult) with no side effects, making them trivially testable and independently deployable.
Production Guardrails
Action.OpenUrlwith missing URL skipped in all strategies (prevents Discord/Slack/Telegram 400 errors)callback_datatruncated by UTF-8 byte length (64B limit), not character countEcosystem Context
adaptive_cardtool, MCP bridge, marker transport, prompt guidance, action routingRelated PRs
Test Plan
pnpm checkpasses (lint + format)/acard testrenders as HTML with inline keyboard/acard testrenders as Block Kit with button/acard testrenders as embed with button component/acard testrenders as native Adaptive Card