add a2ui message rendering#5838
Conversation
| { | ||
| "name": "@tloncorp/api", | ||
| "version": "0.0.6", | ||
| "version": "0.0.8", |
There was a problem hiding this comment.
0.0.7 was published but the version bump wasn't committed
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
@claude review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfd87c1ad4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Two things stand out:
|
Addressed this by making rendering conditional on ability to start a draft in context.
Addressed this by only rendering the blocks in dms. Right now, the only thing buttons can do is send a message with given text in the same context, which is innocuous in a dm. We can consider a more elaborate solution later. |
arthyn
left a comment
There was a problem hiding this comment.
lgtm, do we want to update docs/post-blobs.md? Is the "update surface" message intended to update previous surfaces in the message history or is it just on a per message basis? would love to see more on how that's supposed to work
| const canRenderA2UI = isDmChannelId(post.channelId); | ||
| const canHandleA2UIAction = | ||
| canRenderA2UI && | ||
| !!draftInputContext && | ||
| draftInputContext.canStartDraft !== false; |
There was a problem hiding this comment.
🟡 The DM-only render gate at StaticChatMessage.tsx:122 uses isDmChannelId(post.channelId) only, which matches 1:1 DMs (~-prefixed) but excludes group/multi-DMs (0v-prefixed) — so a2ui cards posted in group DMs are silently filtered to their fallback text, contradicting the documented "render only in direct messages" policy and your stated send-message safety rationale, both of which apply equivalently to multi-DMs. Combined with the activity/reference paths that render the card regardless, a multi-DM a2ui post is visible in activity previews but hidden in the conversation itself. Fix: widen to isDmChannelId(post.channelId) || isGroupDmChannelId(post.channelId) (the established convention used at 6 other callsites in packages/api), or tighten the docs to say "1:1 DMs only" if the narrower scope is intentional.
Extended reasoning...
What's wrong
In packages/app/ui/components/ChatMessage/StaticChatMessage.tsx:122:
const canRenderA2UI = isDmChannelId(post.channelId);isDmChannelId (packages/api/src/client/apiUtils.ts:69-71) returns channelId.startsWith('~') — matching 1:1 DMs only. Group/multi-DMs use a separate 0v-prefixed id detected by the sibling helper isGroupDmChannelId (apiUtils.ts:77-79). The two helpers are mutually exclusive.
Established convention
Every other "is DM-of-any-kind" check in the API package writes both helpers together as a union:
packages/api/src/client/postsApi.ts:229, 314, 684, 812, 1110packages/api/src/client/utils.ts:394
That's 6 callsites consistently using isDmChannelId(channelId) || isGroupDmChannelId(channelId) when they mean "any DM channel". This PR's gate is the lone exception.
How it manifests
canRenderA2UI is used in two places (StaticChatMessage.tsx:130-143 and 216) to:
useMemo-filtera2uiblocks out ofpostContentandlastEditPostContent— so the recipient sees only the inline fallback story text.- Disable
onA2UIActionviacanHandleA2UIAction.
For a multi-DM with channelId like 0v…, both consequences fire: the card is stripped from the message, and even if it weren't, button actions would be inert.
Why the docs and PR rationale don't justify the narrower scope
docs/post-blobs.md(added in this PR): "rendering policy: A2UI blocks render only in direct messages for now" — plural and generic.- Your reply to @patosullivan (2026-05-18T18:15:49Z): "Addressed this by only rendering the blocks in dms. Right now, the only thing buttons can do is send a message with given text in the same context, which is innocuous in a dm." — speaks generically about DMs.
The safety argument — "the button can only send a message in the same context" — applies identically to multi-DMs. sendPostFromDraft posts to draftInputContext.channel.id, whatever its type. A group DM's sender and recipient set are knowable in the same way a 1:1 DM's are. There is no rationale in the PR or docs that distinguishes 1:1 from group DMs.
Step-by-step proof
- Alice posts an a2ui blob to a multi-DM (channelId
0v…) shared with Bob and Carol. validateBlobEntryaccepts the payload,convertContentemits anA2UIBlockData, the post is stored with both a story (inline fallback text) and the a2ui block.- Bob opens the multi-DM.
StaticChatMessagerenderspostwithchannelId=0v…. - Line 122:
canRenderA2UI = isDmChannelId('0v…')→'0v…'.startsWith('~')→false. - Line 132-136:
content = postContent.filter((block) => block.type !== 'a2ui')— the a2ui block is stripped. ChatContentRendererrenders only the fallback paragraph text. Bob sees "Weather: 22903 is 57F and partly cloudy." instead of the weather card.- Meanwhile, the activity feed entry and any quoted reference of this post render through paths that bypass
StaticChatMessage, so the same post's a2ui card does appear in Bob's activity preview / quoted snippet. Bob now has a UX inconsistency: card visible in activity, hidden in chat.
Impact
Not a security or correctness bug — the recipient sees the inline fallback text in the multi-DM, no crash, no data loss. The current behavior is strictly safer than the docs imply. But:
- It contradicts the documented "render only in direct messages" policy.
- It produces an observable UX inconsistency with the activity/reference paths (which render a2ui unconditionally).
- It breaks the codebase convention used at 6 sibling callsites in
packages/api. - The author's own safety rationale doesn't distinguish 1:1 from group DMs.
Severity
nit — real codebase-convention bug, one-line fix, no security impact because the failure mode is conservative (hide the card). Filing for fixability rather than blocking.
How to fix
Either widen the gate (recommended, matches docs + convention):
import { isDmChannelId, isGroupDmChannelId } from '@tloncorp/api/client';
// ...
const canRenderA2UI =
isDmChannelId(post.channelId) || isGroupDmChannelId(post.channelId);Or tighten the docs to "1:1 DMs only" if the narrower scope is intentional. The validator/postContent code is shared across all DM types so widening the gate is the lower-friction option.
| const handleButtonPress = useCallback( | ||
| (component: A2UI.Button) => { | ||
| const fallbackText = | ||
| component.action.event.context?.text ?? | ||
| getComponentText(components.get(component.child), components); | ||
|
|
||
| if (!fallbackText.trim()) { | ||
| return; | ||
| } | ||
|
|
||
| context.onA2UIAction?.(component.action, fallbackText); | ||
| }, | ||
| [components, context] | ||
| ); |
There was a problem hiding this comment.
🟡 A2UI button onPress at A2UIBlock.tsx:148-161 fires context.onA2UIAction?.(...) synchronously and drops the returned promise. The handler (handleA2UIAction in StaticChatMessage.tsx:95-121) is async and awaits draftInputContext.sendPostFromDraft(...), so any rejection becomes an unhandled promise rejection. The dominant network-failure mode is bounded (the optimistic post in _sendPost is marked deliveryStatus: failed, giving the user visible feedback), so impact is limited to pre-send throw paths (e.g. db.getChannel, sync.handleAddPost). One-line fix: attach .catch(logger.error) at the call site, or wrap handleA2UIAction in try/catch — matching the existing pattern at handleRetryPressed in the same file and at BareChatInput/index.tsx:457-465.
Extended reasoning...
What is wrong
handleButtonPress (A2UIBlock.tsx:148-161) is a synchronous useCallback that fires the action and ignores the returned Promise:
const handleButtonPress = useCallback(
(component: A2UI.Button) => {
const fallbackText = ...;
if (!fallbackText.trim()) return;
context.onA2UIAction?.(component.action, fallbackText);
},
[components, context]
);In real usage, context.onA2UIAction is bound to handleA2UIAction (StaticChatMessage.tsx:95-121), which is async and awaits draftInputContext.sendPostFromDraft(...). When the inner call rejects, handleA2UIAction returns a rejected Promise that handleButtonPress discards — an unhandled rejection.
Step-by-step proof
- User taps a Refresh/Allow/etc. button on an A2UI card in a DM.
handleButtonPress(component)invokescontext.onA2UIAction?.(component.action, fallbackText). Because the call is not awaited and no.catchis attached, the returned Promise is unobserved.handleA2UIActionenters itsawait draftInputContext.sendPostFromDraft({...})path. In production this resolves tofinalizeAndSendPost→_sendPost(postActions.ts:141-385)._sendPostdoes most of its work inside a try/catch starting at ~line 270 that marks the optimistic post asdeliveryStatus: failedon send-time errors. But the pre-try setup phase (db.getChannel,db.getContact,db.buildPost,sync.handleAddPostat line 266) is outside that try/catch.- If any of those throws (rare but possible — e.g. SQLite transient failure during the optimistic insert), the rejection propagates back through
handleA2UIActionand into the dropped Promise. React Native logs anUnhandledPromiseRejectionWarning; production gets nothing user-visible.
Why existing safeguards do not fully cover this
_sendPost’s try/catch handles network/send failures by settingdeliveryStatus, so those do surface to the user as a failed post in the chat. This is the refutation’s strongest point and it is correct — the “user gets no feedback” framing overstates the realistic case.- However, pre-send DB/sync throws bypass that try/catch and propagate.
handleRetryPressedin this same file (StaticChatMessage.tsx:87-93) already wrapsonPressRetryin try/catch +console.error.BareChatInput/index.tsx:457-465wraps the analogoussendPostFromDraftcall in try/catch withbareChatInputLogger.error. The PR introduces a third caller that breaks that local convention.
Addressing the refutation
The refutation makes several fair points:
ButtonInput.tsx:33has the identical drop-the-promise pattern. True — and that means this is consistency with one of two existing patterns in the codebase, not a regression. ButBareChatInput(the primary text-send path) andhandleRetryPressed(the same-file analogue) both use try/catch, so the convention is mixed and the more defensive pattern is the one used in the higher-traffic surfaces.- Errors are shown via the optimistic-post pipeline. True for send-time failures. Pre-send throws are the remaining unhandled window and they are rare.
- RN unhandled rejections do not “intermittently crash.” Correct — the bug description’s phrasing was too strong. The real-world consequence is a yellow-box warning in dev and silence in prod, not a crash. Severity adjusted accordingly.
handleRetryPressedhas different semantics. Partially fair — retry has no optimistic-pipeline backing — butBareChatInputdoes have the same optimistic pipeline and still uses try/catch, so the convention argument holds for the closer analogue.
Given the refutation, this is not a blocker. But the fix is one line and removes a known footgun.
How to fix
const handleButtonPress = useCallback(
(component: A2UI.Button) => {
const fallbackText = ...;
if (!fallbackText.trim()) return;
void Promise.resolve(
context.onA2UIAction?.(component.action, fallbackText)
).catch((e) => {
// log via existing logger
console.error("A2UI action failed", e);
});
},
[components, context]
);Or alternatively wrap the body of handleA2UIAction in StaticChatMessage.tsx in try/catch + console.error, matching handleRetryPressed directly above it.
Updated docs -- right now it's just per-message, but we'd like to figure out a way to do live updates in the future. |
Summary
Adds a first pass at A2UI rendering in messages. This keeps the client presentation-only for now, with validation around the small supported subset before we render it.
Changes
@tloncorp/api@0.0.8with the A2UI types and validation helpers.How did I test?
Tested on web + device