fix(web): preserve XML-like tags in user messages - #4133

💭 I see the title attribute issue was already fixed in the current commit, so that's resolved. Now I'm looking at the ChatMarkdown.tsx diff to identify any new problems, particularly around what happens when parseRawHtml is disabled and whether all the rehype plugins are being properly handled. The concern is that disabling rehype plugins entirely might break other functionality like code highlighting or custom styling that user messages depend on. I should examine the actual file to see what's in the plugin list and whether this change affects the UI consistency. With parseRawHtml=false, the sanitize step gets skipped, but markdown-generated HTML is already safe since React escapes it. The sanitize plugin was stripping title attributes, which I'm now handling through custom renderers for links and images. I should check if there are other places where title could appear—fence code blocks and blockquotes can't have titles from markdown alone without raw HTML, so it looks like links are the only concern. React-markdown has its own urlTransform that sanitizes URLs by default in v9, restricting to safe protocols, so even without the sanitize plugin, something like [x](javascript:alert(1)) would be blocked. I need to verify whether ChatMarkdown overrides this urlTransform and check how the a renderer handles the href attribute. Also, when skipHtml=false, raw HTML nodes get rendered as text rather than actual HTML, which is safer—though there's a UI concern if user messages contain literal < characters. I need to check whether the custom markdownUrlTransform function allows JavaScript URLs, and I'm also noting that the sanitize schema permits dataCodeMeta and dataInlineCode attributes on code elements—these get added by remark plugins as hProperties and pass through fine. Additionally, remarkGithubAlerts adds a dataAlert attribute to blockquotes. For the img renderer in assistant messages where parseRawHtml=true, sanitize strips the title attribute from all elements, and the new component drops both node and title—this matches the previous default behavior. In literal mode where parseRawHtml=false, raw HTML in user messages just renders as text, so there's no UI inconsistency there. The real question is how react-markdown handles raw HTML nodes when skipHtml={false} without rehypeRaw—it seems the mdast-to-hast conversion produces raw nodes only when allowDangerousHtml: true, which react-markdown v9 does set in its toHast options. So when a raw HTML block gets converted to a text node at the root level, it sits outside any paragraph wrapper, which means it loses paragraph styling and margins, and the newlines collapse into whitespace—that's a real UI consistency issue where user messages with multi-line HTML content render as unstyled bare text instead of preserving the structure of surrounding paragraphs. I'm checking how the ChatMarkdown component handles whitespace and styling when user messages contain HTML-like content, particularly whether bare text nodes at the root level bypass the paragraph wrapper and its associated CSS margins and formatting rules. Looking at how HTML blocks are handled when rehypeRaw is disabled—the mdast HTML block becomes a hast raw node at the root, which then becomes a text node inside the container div. Since .chat-markdown doesn't have white-space: pre-wrap applied (except in the terminal-context wrapper), newlines within the raw block collapse to spaces, and the text isn't wrapped in a <p> tag, so the paragraph margins don't apply. This is actually a real visual inconsistency introduced by the change. Before, with rehypeRaw enabled, a <div>hello</div> block would render as an actual div element. Now it renders as literal text, which is intentional, but the line collapsing means multi-line raw HTML blocks lose their structure while the rest of the message preserves line breaks through remarkBreaks. For something like issue #4059 with <global-agent-instructions> tags, if a user pastes a block starting at line start with blank line separation, micromark's HTML block rule (type 7 for arbitrary tags) requires the tag to be alone on its line and preceded by a blank line or start of document. The entire block including interior content becomes a single raw node, which renders as one text node at root with newlines collapsed to spaces by CSS — no paragraph margins, and remarkBreaks doesn't apply since it operates on text nodes in mdast, not HTML nodes. That's exactly the repro scenario in the issue, and the PR tests only cover the inline case, so this is a concrete finding worth flagging: literal mode loses line breaks and paragraph typography for HTML blocks. The mdast HTML block value does include newlines, and React renders them, but CSS white-space: normal collapses them — though behavior differs depending on whether the context has whitespace-pre-wrap applied. I should verify the actual issue report to see the exact repro, check whether ChatMarkdown is used elsewhere, and look for existing tests covering multi-line HTML blocks to confirm the escape and render behavior. or sit flush against them depending on the margin collapse rules. Let me trace through the actual rendering to confirm whether the bare text node inherits or collapses margins with adjacent paragraphs. The real question is whether this is a regression from before the PR. Previously, the block-level HTML would've been parsed as an unknown custom element like <global-agent-instructions>, which rehype-sanitize would strip entirely while preserving the inner text content. So the tags disappeared and the text rendered normally. Now the tags are preserved but the newlines inside them collapse to a single line, which is the visual regression we're seeing. This breaks the contract that remarkBreaks establishes for user messages—newlines should be preserved. The HTML block is violating that guarantee. Inline HTML within paragraphs like Hello <tag> wouldn't have this issue since inline elements are typically single-line anyway, but block-level HTML with internal newlines is the problem case here. I'm also thinking through the image renderer and how it handles the transform—that's fine. The a renderer dropping the title attribute for assistant messages is already handled by sanitization, so no accessibility loss there since the href is still visible as a tooltip. Now I need to verify whether skipHtml is even a valid prop in react-markdown v9, since I recall it might have been removed in that version. Let me check the package.json to confirm the react-markdown version, and also look for any existing tests that render ChatMarkdown with block HTML in user messages. I'm checking whether skipHtml is still available in react-markdown v10, since the changelog mentions it was removed. Let me search the codebase for other uses of this option to understand the impact. Looking at the existing tests, there's one that renders unsafe HTML as inert text, but it's a single-line example. I need to verify how react-markdown v10 handles raw HTML nodes when skipHtml is false — specifically whether it converts them to text or leaves them as-is. Checking the v10 source... Writing code snippet... The block-level HTML isn't getting wrapped in paragraph tags like it should be, so it loses the vertical spacing that other elements have. This is a real regression introduced by the parseRawHtml={false} change — before, the tags would strip but the text would still render in paragraphs with proper margins. The simplest fix would be to add a rehype plugin that converts raw HTML nodes into properly structured text nodes that preserve line breaks, or reconsider the sanitization approach. I'm leaning toward the remark plugin approach—converting HTML nodes to text or paragraph nodes before remarkBreaks runs, so block HTML preserves line breaks and spacing. Let me verify that ChatMarkdown isn't preprocessing user message text elsewhere and check whether the raw HTML node actually exists when skipHtml={false} without rehypeRaw. React-markdown passes allowDangerousHtml: true by default in its remarkRehypeOptions, so the HTML should be there. I should also consider that in literal mode, rehypeSanitize doesn't run anymore, meaning anything the remark pipeline produces—like GitHub alerts blockquotes, footnotes, or autolinks from remarkGfm—stays unsanitized, though React's escaping handles most XSS risks except URL protocols, which urlTransform should cover. The urlTransform function is safe to use. I'm thinking through the copy handler behavior with chatMarkdownClipboardPayload — in literal mode, escaped tag text becomes a plain text node, so copying would include the literal <global-agent-instructions> in the markdown output, which might get re-escaped but shouldn't be a problem. I should verify if there are other consumers of ChatMar that I need to account for. I need to check whether other components rendering user text — like message edit previews, user messages in the ChatComposer, task lists, review comment cards, or PlanCard — should also have parseRawHtml={false} for consistency. Let me search for all ChatMarkdown usages to ensure I'm applying this setting uniformly across the codebase. I'm checking for other places where user-typed text gets rendered. There's something at line 1112 in MessagesTimeline I need to look at more closely — I should examine the surrounding context to understand which role is involved. FilePreviewPanel also renders user-authored content through markdown files with HTML parsing enabled, which makes sense for that use case, and PullRequestMarkdown handles PR bodies similarly where HTML rendering is expected. I'm keeping the parseRawHtml default as true for the Assistant row to stay consistent with the original intent. Now I need to check if there are other user-authored surfaces where text should be rendered literally—like in queued message previews or message-edit views. I'm searching through the codebase to see where user text appears, particularly in components like QueuedMessage or UserMessageBody, and tracing how parseRawHtml gets passed to those markdown rendering calls. I'm checking whether the HTML block rendering is actually unsafe—line 1862 shows a code fence with diff content that's already escaped, so that's not a vulnerability. The real issue seems to be that block-level HTML in user messages renders as unwrapped, collapsed text without any protection. Let me verify that mdast treats HTML blocks as root-level nodes per CommonMark spec and check if there's any global CSS that might mitigate this. Looking at the likelihood of hitting this code path: it's very probable when a user pastes multi-line XML or HTML snippets, and the PR's own test even includes a <script> block at the start of a message. I should post an inline comment on the ChatMarkdown component flagging this issue and recommending a fix—either converting mdast HTML nodes to text within a paragraph before the breaks plugin runs, or rendering raw nodes with preserved line structure. I'll also need to add a test case for multi-line HTML blocks in user messages. The tricky part is plugin ordering: if I add a new remark plugin to handle literal HTML, it needs to run before the breaks plugin to work correctly. But I need to be careful about how inline HTML nodes inside paragraphs get handled—converting them to text nodes would make escaping explicit, which is actually fine. Now I'm questioning whether this warrants failing the check. The rules mention that a clear consistency or regression risk can fail it. There's definitely a visual regression here: block HTML in user messages loses its line structure in the changed behavior, which is exactly what the PR targets. But is it actually a regression compared to main? On main, block HTML was parsed and rendered (details/summary elements worked), and unknown tags were dropped with their inner text rendered as children. The paragraph structure was already being collapsed by the rehype-raw and sanitize pipeline, so the newline collapsing for block HTML was already happening there too. So the patch doesn't actually worsen the whitespace handling—it was already collapsed. What it does is make the tags themselves visible, which fixes the original bug. The question is whether making the patch relevant to this pre-existing whitespace issue counts as something I should report. When a user types block HTML with newlines, they'll now see it all on one line with the tags visible. It's the same class of issue that existed before, just now the tags are shown. Given the guidance about reporting concrete violations introduced by changed lines or behavior, this arguably qualifies since the patch makes the raw-to-text conversion behavior directly relevant. But I'm not sure it's a strong enough finding on its own. Let me verify the HTML block parsing rules. For type 7 blocks (any tag), the condition is that the line begins with a complete open or closing tag followed only by whitespace, and it can't interrupt a paragraph. So a message starting with <global-agent-instructions> alone on the first line would start a block that ends at a blank line. But for known block tags like <div>, they can interrupt a paragraph, whereas unknown tags like <config> can't—so they'd stay inline and use line breaks instead. That distinction matters for how the content gets rendered. I'm fairly confident in this finding, though I need to be careful about false positives. The title issue I flagged before was valid and got fixed. Now I'm wondering if there's anything else worth reporting. Let me look at the image renderer—it takes the node and props, including alt, src, and className, and returns an img element. But I should check if there's custom handling elsewhere, like asset URL rewriting through createAssetUrl. I'm checking whether the img renderer might cause React key or props issues — in v10, components receive the node prop, and destructuring that is fine. I'm also verifying that in literal mode, components like MarkdownDetails and tables won't render for user messages since they can't create those elements anyway. There's also the a component that destructures title with an underscore prefix, though that thought seems to cut off mid-sentence. The underscore convention for unused variables is standard, so that's fine. I'm now thinking through potential UI regressions — inline code and fences look okay, and remarkGfm with defaultUrlTransform handles autolinks without sanitization. There's no dangerouslySetInnerHTML being used, which is good. The real concern is block-level HTML whitespace: when a script tag appears as a raw HTML block followed by an image on the same line, it becomes a single HTML node at the root of the markdown div rather than wrapped in a paragraph, so it loses the paragraph styling and inherits typography from the container instead. For single-line content this is mostly acceptable visually, but I'm still evaluating whether this is worth addressing given the multiline case. Since the main branch already collapses this same text (arguably worse, with tags removed entirely), this patch is more of an incomplete fix than a regression. I need to verify main's actual behavior to decide if I should flag this as a failing issue or just note it inline. So the literal mode's rendering of block-level HTML loses typed line breaks—a pre-existing whitespace issue that's now more visible with the patch, but not a regression against main. Looking at the check's criteria around visual and layout preservation, this finding about whitespace and paragraph handling doesn't quite fit the violations it's designed to catch, which focus on shared controls, CSS ownership, and responsive geometry. Given that the prior issue was fixed and this remaining candidate is a non-regression pre-existing behavior, I'm leaning toward clearing this. Let me verify one more thing though: the img renderer applies in both modes, and since title was already stripped in assistant mode via sanitization, there's no change there. For the a renderer dropping title in both modes, that's actually the consistency fix I flagged earlier—now matching assistant behavior. That's good. But I should check if there are any other markdown elements that could produce a title attribute in literal mode. I'm checking whether skipHtml={false} in assistant mode actually matters — since rehypeRaw converts raw tokens to elements before sanitization runs, there shouldn't be any raw nodes left to skip anyway. The whitespace rendering gap seems like the only concrete finding here, so I should verify that claim more carefully by tracing through how UserMessageBody actually handles the content. I'm deciding whether to include a comment about block-level HTML whitespace handling. The existing code already collapses those newlines, and there's a principle against turning a focused PR into unrelated changes, but the core purpose here is faithful rendering of user input—and block HTML with multiple lines is a realistic scenario worth handling correctly. The real issue is inconsistency: in the plain path, whitespace gets collapsed, but in the terminal-context path with whitespace-pre-wrap, the same message preserves newlines. That's a genuine UI consistency problem, though it's pre-existing for markdown content too. I need to verify whether block-level HTML nodes actually end up wrapped in paragraphs by checking the test expectations and tracing through how micromark and mdast handle raw HTML input. I'm confident in the technical analysis—multi-line blocks become text nodes with newlines that collapse in HTML rendering. The question is whether this warrants a comment. It's an incomplete-fidelity issue rather than a hard regression, but if I'm posting a finding, I should mark the check as failed and include it in the summary. Let me verify the actual impact: if someone pastes block-level HTML like <global-agent-instructions>, the rendered output becomes a single collapsed line, which breaks the line fidelity that markdown normally preserves. That's worth reporting. I'll add a comment on the rehypePlugins configuration line in the diff. because remarkBreaks only affects text nodes. The fix would be to add a remark plugin in literal mode, placed before remarkBreaks, that converts mdast html nodes into text nodes and wraps root-level ones in paragraphs — this way the escaped content gets proper line breaks and spacing like the rest of the message. I should also note the inconsistency with the terminal-context path, which does use white-space: pre-wrap. This is a multi-hunk change, so no inline suggestion. I'll post it as a comment via the GitHub PR review API. Given the concrete finding here, I think this warrants a failure state rather than just a comment, even if it's more of a completeness issue. The check defaults to failure anyway, so I'll go with that and write up the comment concisely.

1. modify_pr — One finding on the literal-HTML rendering path: block-level HTML in user messages now renders as an unwrapped, whitespace-collapsed text run. The title issue flagged on the previous commit is resolved by the new a/img renderers.