Skip to content

fix(chat): a non-text message type rendered an empty bubble — the approval card's fallback text was dropped (TASK-056) - #1758

Merged
lilyshen0722 merged 2 commits into
mainfrom
kai/task056-card-fallback-text
Sep 19, 2026
Merged

lilyshen0722 merged 2 commits into
mainfrom
kai/task056-card-fallback-text

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Fixes TASK-056 (C4-9): a member saw "C4 smoke · less than a minute ago" over an empty bubble while the card was visible to the approver.

What I could and could not measure

I could not reproduce this live: the C4 run room is not one of my pods, the cluster is unreachable from here (no gke-gcloud-auth-plugin), and I must not post as a human. So this is a code read, and I have separated what it settles from what it does not.

Settled. The approval card is posted with message_type: 'card' (backend/services/approvalActionService.ts:365) and content: '[approval needed] ' + summary, deliberately written as "the plain-text fallback so legacy surfaces (v1, digests, notifications) still show something meaningful". In frontend/src/components/ChatRoom.tsx the generic bubble rendered a body only for messageType === 'text' (and 'image'), and that component has no payload renderer at all — ADR-020 D3's card is v2-only. system returns earlier, 'card' matches nothing, so the message rendered an author line, a timestamp and no body. That is the reported symptom, in a component that is supposed to be its fallback consumer.

Settled — the discriminator is the SURFACE, not the role. Vera checked the routing rather than reasoning about it, and I re-derived both halves myself: frontend/src/v2/V2App.tsx:237 mounts pods/:podIdV2PodIdRoute, while :239 (pods/:podType/:roomId) and :252 (chat/:podId) still mount the legacy ChatRoom. The stranger-view capture in #1749 (t3-create-card-stranger-view.png) shows the composer reading "Message #C4 run 2026-09-18" — a placeholder that exists in exactly one place in the tree (grep -rn 'Message #' frontend/srcChatRoom.tsx:4894, Message #${room?.name || 'chat'}). So the stranger was on the legacy renderer, reachable from the v2 shell through the two-segment pod route, and the v2 path is not broken for a non-owner (V2MessageRow renders V2ApprovalCard for any viewer when payload.kind === 'approval-card'; the non-owner branch is asserted at V2ApprovalCardAction.test.tsx:178).

That widens the defect rather than narrowing it, and it is why this PR is worth more than a non-approver's view: the empty body came from the message type, not the viewer, so on this surface every reader gets it — including the person who is supposed to approve. The failure is not "a non-approver cannot see the arguments"; it is "a card posted to a legacy room is invisible to everyone". Residual, named and out of scope: the legacy surface still offers no approve/decline action, so a legacy-surface approver now reads that something needs approval but cannot act from there — the same class, one step further along, and a v2-link or a legacy card control rather than a text fallback.

The fix

showTextBody = messageType === 'text' || (messageType !== 'image' && !!messageContent.trim()). The text path is unchanged, image keeps its own branch, system still returns above, and no type gains a second renderer — I enumerated the branches in this component: only system, text and image exist, so a widened gate cannot double-render one of them. The body is the server's fallback string, verbatim: a human being asked to approve something now reads that something is being asked, on the surface that is the fallback's only reader.

Mutation-proven: restoring {messageType === 'text' && ( reds exactly the new case and nothing else (1 failed / 8 passed).

  • Frontend 108 suites / 885 tests green, tsc --noEmit clean.
  • Frontend-only, one component hunk + one test; no version bump (the guard watches cli/src and commonly-mcp/src).

Related, and deliberately untouched

The stored card message's counterpart on the other legacy surface is the row's own suggestion — "at least read awaiting a person's approval". The fallback text says [approval needed], so the requirement is met without inventing a second wording. If the pod would rather the legacy bubble carry richer phrasing, that is a wording decision, not this defect.

The approval card posts `message_type: 'card'` and this bubble returned a
body only for `text` (or `image`), so a member reading the room here saw
the author and a timestamp over nothing. `content` is written as the
plain-text fallback for exactly these legacy surfaces — the card
component is v2-only — so render it. TASK-056.
…-056)

Wren's read of the pair is on the PR; this commits the files it read.

- before = main dbd5270, after = dbd5270 + this PR's head a740d5f
- surface: the legacy ChatRoom at /pods/team/6aace2b11df344bc6ccdf0ef, one seeded PG
  card message (`message_type='card'`, `[approval needed] …`), one backend at main
- harness as in #1770, so the 390 files are 780x1800 rather than an unlabelled 1440

Measured: the bubble under the author line gains the fallback body at both widths —
807->863ch at 390, 819->875ch at 1200.
@lilyshen0722
lilyshen0722 force-pushed the kai/task056-card-fallback-text branch from 6bb5ce8 to c340541 Compare September 19, 2026 17:27
@lilyshen0722
lilyshen0722 merged commit 6113320 into main Sep 19, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant