feat(chat): teams message type coverage, attachment mentions, in-app message links (t181-t184) - #49
Merged
Merged
Conversation
Dokploy Preview Deployment
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Four tasks that together close the gap between what Teams web displays and what CDP Chats displays, plus the two bugs found while testing it.
A live probe of 1963 raw Teams messages across 20 conversations measured what the renderer was actually dropping, rather than guessing from the reported samples. Every payload was replayed through the real
core/teams-render.jsand diffed raw-vs-rendered, which is how the largest gap (206 messages) surfaced — nobody had reported it.t181 — message-type coverage
Two payload facts worth recording, both non-obvious:
properties.files, not as an inline<img>— a completely different code path from a pasted image, which is why it degraded to a chip. Its preview is AMS-hosted, so the existingisValidAmsUrlproxy already covers it; the SharePointshareUrlon the same object is not proxyable and stays a link-out.properties.cards[].content.componentUrl. That URL is third-party, so it is scheme-guarded to http(s) before it reaches anhref(see Security below).t182 — mentions and quotes through attachments
Attaching a file silently dropped every mention and reply quote on the message. The composer collected them, then the frontend explicitly zeroed them out — because the upload signature had nowhere to put them (
text?: stringand nothing else).The fix threads an
UploadOpts({text, html, quotes, mentions}) through the full stack and gives the Teamspropertiespayload a single owner: the new purecore/teams-send-props.js, shared by all four senders. Previously onlyteamsReplybuilt that blob, which is the structural reason the three upload paths couldn't carry mentions.Two details that are load-bearing:
carriesCaptioninchat/src/lib/send-chain.ts). Keying on the loop index meant a failing first upload took the caption, mentions and quotes down with it — the same class of silent drop this task exists to remove. Found while writing the task's own tests.t183 — message links resolve in-app
A Teams
/l/message/{convId}/{msgId}permalink, or one of our own/chat/c/{convId}?msg=links, now jumps inside the app. Only the real Teams hosts and our own origin qualify — a foreign origin's/chat/c/is somebody else's app. Modifier clicks pass through untouched, so ⌘-click still opens a tab.In-app links also look different (dotted underline + a
↩marker). A link that behaves differently from every other link has to signal that; the marker is a CSS::afterso it never lands in copied text or a screen reader.t184 — jump landing holds through media load
Jumping into a thread full of images landed correctly and then drifted, because media above the target has no reserved box and grows when it decodes. The landing now re-seats on every in-pane media load for a 3s settle window.
jumpToUnreadalready had this treatment; the message jump did not.Security
The mutation check earned its keep:
componentUrlcomes from a third-party Fluid card and flowed straight into a chiphrefwith no scheme guard. Ajavascript:payload would have ridden through. Guard added, with a test coveringjavascript:,data:andfile:.chat/src/lib/sanitize-message.tsis unchanged — every tag and attribute involved was already allowlisted, so no new XSS surface.Verification
pnpm test— 2538 passed (186 files); typecheck, build and Biome cleanscripts/t181-corpus-regression.mjs): the 1963-message probe corpus replayed through the renderer, all five gap classes report 0naturalWidth, lightboxrole="dialog",defaultPreventedper click type, scroll offsets before/after settle)git bisectstays meaningfulKnown ceiling
store.tspersists the rendered message body, so a renderer fix only reaches messages that get re-fetched — the newest ~30 of a thread. Older DB-served pages keep the old body until they're re-swept. A backfill pass is logged indocs/memories/ideas.mdalongside four other deliberate deferrals (link unfurls, call transcripts,<cite>styling, mentions in edits), each with the reason it was skipped.