From 3cf116f6a103d6597c8575f14fb21f1cc1bfea41 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Fri, 17 Jul 2026 17:09:04 +0800 Subject: [PATCH 1/5] refactor(markdown): single canonical sanitize schema for both renderers (MUL-4922) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 of the RichContent convergence: collapse the duplicated security base shared by the two product-level Markdown chains. Chat (packages/ui/markdown/Markdown.tsx) and Issue/Comment (packages/views/editor/readonly-content.tsx) each carried a verbatim fork of the rehype-sanitize schema and urlTransform, and the forks had already drifted: readonly whitelisted for `==highlight==`, chat did not. A security-relevant allow-list maintained in two places means every future XSS fix has to land twice, and missing one is a hole — this is the hardest reason for the sweep, ahead of the user-visible feature drift. - Extract markdownSanitizeSchema + markdownUrlTransform into packages/ui/markdown/sanitize.ts and export from the package index. Both chains now import the single copy; no local forks remain. - The canonical schema is the union, so chat gains the tag name. This is the one intentional behavior delta: is inert and admits no attributes, and chat needs it anyway once ==highlight== converges. - Annotate the schema as rehype-sanitize's Options: exporting it makes the previously-inferred hast-util-sanitize type unnameable across packages. Adds a cross-surface contract test that runs one set of security fixtures (script, event handlers, javascript: href, data:image vs data:text/html, mark, slash://) through BOTH surfaces and asserts identical outcomes — the mechanism that stops a third fork from growing back. Code-block rendering is deliberately not asserted cross-surface yet: chat highlights with Shiki, readonly with lowlight, so emitted class tokens still differ. Converging them is the RichCodeBlock phase and needs the highlight-engine decision first; only the schema-level allow-list is shared here. Verified: pnpm typecheck (6 workspaces), pnpm lint (0 errors), pnpm vitest run in packages/views (228 files, 2665 tests, all passing). Co-authored-by: multica-agent Co-Authored-By: Claude Opus 4.8 --- packages/ui/markdown/Markdown.tsx | 67 +------ packages/ui/markdown/index.ts | 1 + packages/ui/markdown/sanitize.ts | 75 +++++++ .../rich-content-sanitize-contract.test.tsx | 184 ++++++++++++++++++ packages/views/editor/readonly-content.tsx | 80 +------- 5 files changed, 275 insertions(+), 132 deletions(-) create mode 100644 packages/ui/markdown/sanitize.ts create mode 100644 packages/views/common/rich-content-sanitize-contract.test.tsx diff --git a/packages/ui/markdown/Markdown.tsx b/packages/ui/markdown/Markdown.tsx index 8c9651e5454..beaa5d53e85 100644 --- a/packages/ui/markdown/Markdown.tsx +++ b/packages/ui/markdown/Markdown.tsx @@ -1,8 +1,8 @@ import * as React from 'react' -import ReactMarkdown, { type Components, defaultUrlTransform } from 'react-markdown' +import ReactMarkdown, { type Components } from 'react-markdown' import rehypeKatex from 'rehype-katex' import rehypeRaw from 'rehype-raw' -import rehypeSanitize, { defaultSchema } from 'rehype-sanitize' +import rehypeSanitize from 'rehype-sanitize' import remarkBreaks from 'remark-breaks' import remarkGfm from 'remark-gfm' import remarkMath from 'remark-math' @@ -14,6 +14,7 @@ import { isAllowedFileCardHref, preprocessFileCards } from './file-cards' import { preprocessLinks } from './linkify' import { preprocessIssueIdentifiers } from './issue-identifiers' import { preprocessMentionShortcodes } from './mentions' +import { markdownSanitizeSchema, markdownUrlTransform } from './sanitize' import 'katex/dist/katex.min.css' import './markdown.css' @@ -87,64 +88,6 @@ export interface MarkdownProps { autolinkIssueIdentifiers?: boolean } -// Sanitization schema — extends GitHub defaults to allow code highlighting classes -// and Multica's internal mention/slash protocols. -const sanitizeSchema = { - ...defaultSchema, - protocols: { - ...defaultSchema.protocols, - href: [...(defaultSchema.protocols?.href ?? []), 'mention', 'slash'], - // Permit inline data-URI images (QR codes, charts, base64 screenshots). - // The scheme gate only allows `data:` through here; attributes.img below - // narrows it to image/* so non-image data URIs are still rejected. - src: [...(defaultSchema.protocols?.src ?? []), 'data'], - }, - attributes: { - ...defaultSchema.attributes, - div: [ - ...(defaultSchema.attributes?.div ?? []), - 'dataType', - 'dataHref', - 'dataFilename', - ], - code: [ - ...(defaultSchema.attributes?.code ?? []), - ['className', /^language-/], - ['className', /^math-/], - ['className', /^hljs/], - ], - img: [ - // Drop the default plain `src` entry so the value allow-list below is the - // one findDefinition resolves — it returns the first match by name, so a - // bare `src` string would otherwise shadow (and disable) the allow-list. - ...(defaultSchema.attributes?.img ?? []).filter( - (attr) => (typeof attr === 'string' ? attr : attr[0]) !== 'src', - ), - 'alt', - // Allow inline data:image/* URIs while leaving every other src form - // (http/https/site-relative) exactly as before: the negative lookahead - // keeps all non-data values, and data: is narrowed to images only. - ['src', /^data:image\//i, /^(?!data:)/i], - ], - }, -} - -/** - * Custom URL transform that allows Multica internal protocols while keeping - * the default security for all other URLs. - */ -function urlTransform(url: string): string { - if (url.startsWith('mention://')) return url - if (url.startsWith('slash://skill/')) return url - // Allow inline data:image/* URIs — defaultUrlTransform strips every data: URL - // to '', which would blank the src even after rehype-sanitize keeps it. Kept - // in sync with the image/* narrowing in sanitizeSchema (protocols.src + - // attributes.img) so both gates agree on what a valid inline image is. - if (/^data:image\//i.test(url)) return url - return defaultUrlTransform(url) -} - - // File path detection regex - matches paths starting with /, ~/, or ./ const FILE_PATH_REGEX = /^(?:\/|~\/|\.\/)[\w\-./@]+\.(?:ts|tsx|js|jsx|mjs|cjs|md|json|yaml|yml|py|go|rs|css|scss|less|html|htm|txt|log|sh|bash|zsh|swift|kt|java|c|cpp|h|hpp|rb|php|xml|toml|ini|cfg|conf|env|sql|graphql|vue|svelte|astro|prisma)$/i @@ -483,8 +426,8 @@ export function Markdown({ remarkBreaks, [remarkGfm, { singleTilde: false }], ]} - rehypePlugins={[rehypeRaw, [rehypeSanitize, sanitizeSchema], rehypeKatex]} - urlTransform={urlTransform} + rehypePlugins={[rehypeRaw, [rehypeSanitize, markdownSanitizeSchema], rehypeKatex]} + urlTransform={markdownUrlTransform} components={components} > {processedContent} diff --git a/packages/ui/markdown/index.ts b/packages/ui/markdown/index.ts index ce609eb5d5f..a800eb5091c 100644 --- a/packages/ui/markdown/index.ts +++ b/packages/ui/markdown/index.ts @@ -8,6 +8,7 @@ export { ISSUE_IDENTIFIER_PATTERN, } from './issue-identifiers' export { preprocessMentionShortcodes } from './mentions' +export { markdownSanitizeSchema, markdownUrlTransform } from './sanitize' export { preprocessFileCards, isCdnUrl, diff --git a/packages/ui/markdown/sanitize.ts b/packages/ui/markdown/sanitize.ts new file mode 100644 index 00000000000..bc828d1042d --- /dev/null +++ b/packages/ui/markdown/sanitize.ts @@ -0,0 +1,75 @@ +import { defaultUrlTransform } from 'react-markdown' +import { defaultSchema, type Options } from 'rehype-sanitize' + +/** + * Canonical sanitize schema + URL transform for every product-level Markdown + * renderer (Chat via ui/markdown/Markdown.tsx, Issue/Comment via + * views/editor/readonly-content.tsx). + * + * This is the ONLY copy. Both renderers previously carried a verbatim fork of + * this schema, which had already drifted — the readonly fork whitelisted + * for `==highlight==` and the chat fork did not. A security-relevant allow-list + * maintained in two places means any future XSS fix has to land twice, and + * missing one is a hole. Adding a surface means importing this, never copying + * it (MUL-4922). + * + * The two gates below must agree on what a valid inline image is: + * `protocols.src` + `attributes.img` (sanitize) and `markdownUrlTransform` + * (react-markdown). Change them together. + */ +export const markdownSanitizeSchema: Options = { + ...defaultSchema, + // Allow (text highlight) — emitted by highlightToHtml from `==text==`. + // It carries no attributes, so only the tag name needs whitelisting. + tagNames: [...(defaultSchema.tagNames ?? []), 'mark'], + protocols: { + ...defaultSchema.protocols, + href: [...(defaultSchema.protocols?.href ?? []), 'mention', 'slash'], + // Permit inline data-URI images (QR codes, charts, base64 screenshots). + // The scheme gate only allows `data:` through here; attributes.img below + // narrows it to image/* so non-image data URIs are still rejected. + src: [...(defaultSchema.protocols?.src ?? []), 'data'], + }, + attributes: { + ...defaultSchema.attributes, + div: [ + ...(defaultSchema.attributes?.div ?? []), + 'dataType', + 'dataHref', + 'dataFilename', + ], + code: [ + ...(defaultSchema.attributes?.code ?? []), + ['className', /^language-/], + ['className', /^math-/], + ['className', /^hljs/], + ], + img: [ + // Drop the default plain `src` entry so the value allow-list below is the + // one findDefinition resolves — it returns the first match by name, so a + // bare `src` string would otherwise shadow (and disable) the allow-list. + ...(defaultSchema.attributes?.img ?? []).filter( + (attr) => (typeof attr === 'string' ? attr : attr[0]) !== 'src', + ), + 'alt', + // Allow inline data:image/* URIs while leaving every other src form + // (http/https/site-relative) exactly as before: the negative lookahead + // keeps all non-data values, and data: is narrowed to images only. + ['src', /^data:image\//i, /^(?!data:)/i], + ], + }, +} + +/** + * Allows Multica internal protocols and inline images through react-markdown's + * URL gate while keeping default security for everything else. + */ +export function markdownUrlTransform(url: string): string { + if (url.startsWith('mention://')) return url + if (url.startsWith('slash://skill/')) return url + // defaultUrlTransform strips every data: URL to '', which would blank the src + // even after rehype-sanitize keeps it. Kept in sync with the image/* narrowing + // in markdownSanitizeSchema so both gates agree on what a valid inline image is. + if (/^data:image\//i.test(url)) return url + return defaultUrlTransform(url) +} diff --git a/packages/views/common/rich-content-sanitize-contract.test.tsx b/packages/views/common/rich-content-sanitize-contract.test.tsx new file mode 100644 index 00000000000..12a4de56101 --- /dev/null +++ b/packages/views/common/rich-content-sanitize-contract.test.tsx @@ -0,0 +1,184 @@ +/** + * Cross-surface sanitize contract (MUL-4922). + * + * The two product-level Markdown chains — Chat (ui/markdown/Markdown.tsx) and + * Issue/Comment (views/editor/readonly-content.tsx) — used to carry a verbatim + * fork of the sanitize schema and urlTransform each, and had already drifted: + * readonly whitelisted , chat did not. Both now import the single + * canonical base from @multica/ui/markdown. + * + * This suite runs one set of security fixtures through BOTH surfaces and + * asserts the same outcome. It is the mechanism that stops a third fork from + * growing back: a schema change that lands on only one chain fails here. + */ +import { describe, expect, it, vi } from "vitest"; +import { render } from "@testing-library/react"; +import { + Markdown as MarkdownBase, + markdownSanitizeSchema, +} from "@multica/ui/markdown"; + +vi.mock("@multica/core/config", () => ({ + useConfigStore: (selector: (state: { cdnDomain: string }) => unknown) => + selector({ cdnDomain: "" }), + configStore: { getState: () => ({ cdnDomain: "" }) }, +})); + +vi.mock("../issues/hooks", () => ({ + useResolveIssueIdentifier: () => null, +})); + +vi.mock("../i18n", async () => { + const editor = (await import("../locales/en/editor.json")).default; + return { + useT: () => ({ + t: (select: (bundle: typeof editor) => string) => select(editor), + }), + useTimeAgo: () => "just now", + }; +}); + +vi.mock("@multica/core/api", () => ({ + api: { getAttachmentTextContent: vi.fn() }, + PreviewTooLargeError: class extends Error {}, + PreviewUnsupportedError: class extends Error {}, +})); + +vi.mock("@multica/core/paths", () => ({ + useWorkspacePaths: () => ({ + issueDetail: (id: string) => `/test/issues/${id}`, + projectDetail: (id: string) => `/test/projects/${id}`, + }), + useWorkspaceSlug: () => "test", +})); + +vi.mock("../navigation", () => ({ + useNavigation: () => ({ push: vi.fn(), openInNewTab: vi.fn() }), + AppLink: ({ href, children }: { href: string; children: React.ReactNode }) => ( + {children} + ), +})); + +vi.mock("../issues/components/issue-mention-card", () => ({ + IssueMentionCard: ({ issueId }: { issueId: string }) => {issueId}, +})); + +vi.mock("../projects/components/project-chip", () => ({ + ProjectChip: ({ projectId }: { projectId: string }) => {projectId}, +})); + +vi.mock("../editor/link-hover-card", () => ({ + useLinkHover: () => ({}), + LinkHoverCard: () => null, +})); + +vi.mock("../editor/utils/link-handler", () => ({ + openLink: vi.fn(), + isMentionHref: (href?: string) => Boolean(href?.startsWith("mention://")), +})); + +// Expose the sanitized url/filename that survived the schema, so image +// fixtures can be asserted identically on both surfaces (readonly routes +// images through , the ui base renders a plain ). +vi.mock("../editor/attachment", () => ({ + Attachment: ({ + attachment, + }: { + attachment: { url: string; filename: string }; + }) => {attachment.filename}, +})); + +import { ReadonlyContent } from "../editor/readonly-content"; + +const PNG_1X1 = + "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg=="; + +const SURFACES: ReadonlyArray<{ + name: string; + render: (markdown: string) => HTMLElement; +}> = [ + { + name: "Chat (ui/markdown)", + render: (markdown) => render({markdown}).container, + }, + { + name: "Issue/Comment (ReadonlyContent)", + render: (markdown) => render().container, + }, +]; + +describe.each(SURFACES)("sanitize contract — $name", ({ render: renderSurface }) => { + it("strips "); + + expect(container.querySelector("script")).toBeNull(); + expect(container.textContent).not.toContain("alert(1)"); + }); + + it("strips event-handler attributes from raw HTML", () => { + const container = renderSurface(''); + + expect(container.querySelector("[onerror]")).toBeNull(); + expect(container.innerHTML).not.toContain("onerror"); + }); + + it("neutralizes javascript: hrefs", () => { + const container = renderSurface("[click](javascript:alert(1))"); + + const href = container.querySelector("a")?.getAttribute("href") ?? ""; + expect(href.toLowerCase()).not.toContain("javascript:"); + }); + + it("preserves an inline data:image/* src", () => { + const container = renderSurface(`![demo](${PNG_1X1})`); + + expect(container.querySelector("img")).toHaveAttribute("src", PNG_1X1); + }); + + it("blanks a non-image data: URI", () => { + const container = renderSurface("![x](data:text/html,)"); + + expect(container.querySelector("img")?.getAttribute("src") ?? "").toBe(""); + }); + + it("keeps http(s) image src intact", () => { + const container = renderSurface("![cat](https://cdn.example.com/cat.png)"); + + expect(container.querySelector("img")).toHaveAttribute( + "src", + "https://cdn.example.com/cat.png", + ); + }); + + // The drift this sweep closes: was whitelisted on readonly only, so + // the same content highlighted in a comment and rendered in chat disagreed. + it("allows (==highlight== lowering target)", () => { + const container = renderSurface("hi"); + + expect(container.querySelector("mark")?.textContent).toBe("hi"); + }); + + it("allows the slash:// protocol", () => { + const container = renderSurface("[/deploy](slash://skill/abc-123)"); + + expect(container.querySelector(".slash-command")?.textContent).toBe("/deploy"); + }); +}); + +// Code-block *rendering* is deliberately not asserted cross-surface yet: chat +// highlights with Shiki and readonly with lowlight, so the emitted class tokens +// still differ. Converging them is the RichCodeBlock phase of MUL-4922; until +// the highlight engine is picked, only the schema-level allow-list is shared. +describe("canonical sanitize schema", () => { + it("permits language-/math-/hljs class tokens on ", () => { + const codeAttrs = markdownSanitizeSchema.attributes?.code ?? []; + const patterns = codeAttrs + .filter((attr): attr is [string, RegExp] => Array.isArray(attr)) + .filter(([name]) => name === "className") + .map(([, pattern]) => pattern.source); + + expect(patterns).toEqual( + expect.arrayContaining(["^language-", "^math-", "^hljs"]), + ); + }); +}); diff --git a/packages/views/editor/readonly-content.tsx b/packages/views/editor/readonly-content.tsx index 703dcb5842b..e7aa2f82128 100644 --- a/packages/views/editor/readonly-content.tsx +++ b/packages/views/editor/readonly-content.tsx @@ -17,17 +17,14 @@ */ import { isValidElement, memo, useMemo, useRef, useState } from "react"; -import ReactMarkdown, { - defaultUrlTransform, - type Components, -} from "react-markdown"; +import ReactMarkdown, { type Components } from "react-markdown"; import type { ReactNode } from "react"; import rehypeKatex from "rehype-katex"; import remarkBreaks from "remark-breaks"; import remarkGfm from "remark-gfm"; import remarkMath from "remark-math"; import rehypeRaw from "rehype-raw"; -import rehypeSanitize, { defaultSchema } from "rehype-sanitize"; +import rehypeSanitize from "rehype-sanitize"; import { toHtml } from "hast-util-to-html"; import { Check, Copy } from "lucide-react"; import { cn } from "@multica/ui/lib/utils"; @@ -41,7 +38,12 @@ import { useResolveIssueIdentifier } from "../issues/hooks"; import { ProjectChip } from "../projects/components/project-chip"; import { useLinkHover, LinkHoverCard } from "./link-hover-card"; import { openLink, isMentionHref } from "./utils/link-handler"; -import { isAllowedFileCardHref, isIssueIdentifier } from "@multica/ui/markdown"; +import { + isAllowedFileCardHref, + isIssueIdentifier, + markdownSanitizeSchema, + markdownUrlTransform, +} from "@multica/ui/markdown"; import { preprocessMarkdown } from "./utils/preprocess"; import { highlightToHtml } from "./utils/highlight-markdown"; import { MermaidDiagram } from "./mermaid-diagram"; @@ -59,68 +61,6 @@ import "./styles/index.css"; // don't accidentally match and lose their
 wrapper.
 const PRE_UNWRAP_RE = /(^|\s)language-(html|mermaid)(\s|$)/;
 
-// ---------------------------------------------------------------------------
-// Sanitization schema — extends GitHub defaults to allow file-card data attrs
-// ---------------------------------------------------------------------------
-
-const sanitizeSchema = {
-  ...defaultSchema,
-  // Allow  (text highlight) — emitted by highlightToHtml from `==text==`.
-  // It carries no attributes, so only the tag name needs whitelisting.
-  tagNames: [...(defaultSchema.tagNames ?? []), "mark"],
-  protocols: {
-    ...defaultSchema.protocols,
-    href: [...(defaultSchema.protocols?.href ?? []), "mention", "slash"],
-    // Permit inline data-URI images (QR codes, charts, base64 screenshots).
-    // The scheme gate only allows `data:` through here; attributes.img below
-    // narrows it to image/* so non-image data URIs are still rejected.
-    src: [...(defaultSchema.protocols?.src ?? []), "data"],
-  },
-  attributes: {
-    ...defaultSchema.attributes,
-    div: [
-      ...(defaultSchema.attributes?.div ?? []),
-      "dataType",
-      "dataHref",
-      "dataFilename",
-    ],
-    code: [
-      ...(defaultSchema.attributes?.code ?? []),
-      ["className", /^language-/],
-      ["className", /^math-/],
-      ["className", /^hljs/],
-    ],
-    img: [
-      // Drop the default plain `src` entry so the value allow-list below is the
-      // one findDefinition resolves — it returns the first match by name, so a
-      // bare `src` string would otherwise shadow (and disable) the allow-list.
-      ...(defaultSchema.attributes?.img ?? []).filter(
-        (attr) => (typeof attr === "string" ? attr : attr[0]) !== "src",
-      ),
-      "alt",
-      // Allow inline data:image/* URIs while leaving every other src form
-      // (http/https/site-relative) exactly as before: the negative lookahead
-      // keeps all non-data values, and data: is narrowed to images only.
-      ["src", /^data:image\//i, /^(?!data:)/i],
-    ],
-  },
-};
-
-// ---------------------------------------------------------------------------
-// URL transform — allow mention:// protocol through react-markdown's sanitizer
-// ---------------------------------------------------------------------------
-
-function urlTransform(url: string): string {
-  if (url.startsWith("mention://")) return url;
-  if (url.startsWith("slash://skill/")) return url;
-  // Allow inline data:image/* URIs — defaultUrlTransform strips every data: URL
-  // to '', which would blank the src even after rehype-sanitize keeps it. Kept
-  // in sync with the image/* narrowing in sanitizeSchema (protocols.src +
-  // attributes.img) so both gates agree on what a valid inline image is.
-  if (/^data:image\//i.test(url)) return url;
-  return defaultUrlTransform(url);
-}
-
 // ---------------------------------------------------------------------------
 // Custom react-markdown components
 // ---------------------------------------------------------------------------
@@ -480,8 +420,8 @@ export const ReadonlyContent = memo(function ReadonlyContent({
           remarkBreaks,
           [remarkGfm, { singleTilde: false }],
         ]}
-        rehypePlugins={[rehypeRaw, [rehypeSanitize, sanitizeSchema], rehypeKatex]}
-        urlTransform={urlTransform}
+        rehypePlugins={[rehypeRaw, [rehypeSanitize, markdownSanitizeSchema], rehypeKatex]}
+        urlTransform={markdownUrlTransform}
         components={components}
       >
         {processed}

From 2a1bc20ac6bd7c94a7117bd3a16a1978f08db248 Mon Sep 17 00:00:00 2001
From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com>
Date: Mon, 20 Jul 2026 13:34:22 +0800
Subject: [PATCH 2/5] feat(rich-content): one RichContent renderer for Chat and
 Issue/Comment (MUL-4922)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 2+3 of the convergence. Chat now renders agent output through the same
product renderer as Issue descriptions and Comments, so a ```mermaid fence an
agent emits is a diagram in Chat — not a dead code block.

Before this there were two product Markdown chains. Issue/Comment had Mermaid,
HTML preview, lowlight code, product Mention/Attachment/LinkHover; Chat went
through a generic renderer with no Mermaid/HTML dispatcher at all. Chat is
where agents emit the most diagrams, so the gap was most visible exactly where
it mattered.

Architecture
- packages/views/rich-content/ holds the ONE product renderer: a single
  ReactMarkdown pipeline, one sanitize config, one components map, one fenced
  -code dispatcher. Public API is content/attachments/density/phase — no
  `surface` prop, no renderMention override, no custom code-renderer hook,
  because each is a door a per-surface fork walks back through.
- `density` is CSS only; `phase` is lifecycle only. Neither switches parser,
  plugins or the semantic DOM, so all surfaces emit the same blocks.
- ReadonlyContent is now a ~40-line compatibility wrapper (was 501 lines);
  its consumers are untouched. views/common/markdown.tsx is deleted.
- Leaf components (Mermaid, HTML preview, lowlight code) stay in editor/ and
  are imported by direct path, so Tiptap keeps reusing them and Chat does not
  pull in the editor's Tiptap graph. Moving them is a later mechanical commit.

Streaming fence gate
Rich blocks upgrade only when the fence is CLOSED, judged by a real CommonMark
parse (mdast) rather than a `startsWith("```")` scan. A half-streamed fence
renders as source, so Mermaid never parses a partial diagram and no iframe is
created for HTML still arriving. Closedness — not `phase` — is the gate, so a
settled-but-malformed fence stays source too. The gate returns offsets only; it
never renders, because a gate that rendered would be the second renderer this
change exists to delete. Verified that source offsets survive rehype-raw +
sanitize + katex before relying on them.

Live -> persisted row identity
The live timeline moved out of Virtuoso's Footer into a real row keyed
`task:`, and persisted assistant rows key on the same task instead of
`message.id`. Completion is now an in-place data swap through one
AssistantMessage component, so an already-rendered diagram or iframe stays
mounted and is not re-run. The process fold previously relied on that remount
to re-collapse, so the collapse is now expressed directly.

Notes
- Chat code blocks move from Shiki to lowlight (Howard's ratified choice),
  matching Tiptap/Readonly and the existing hljs CSS. Ligature suppression
  carries over via code.css instead of utility classes.
- Chat message tests now stub react-virtuoso: real Virtuoso renders its Footer
  but no data rows under jsdom's zero-height viewport, and the live timeline is
  a row now.

Tests: five-surface Mermaid parity (Chat user / live / persisted, Issue
description, Comment), streaming open->closed->settled with mount counting,
live->persisted no-remount, htmlbars/mermaidx not misdispatched, sandbox and
data-URI security regressions, CommonMark fence edge cases (shorter closer,
tilde, indented, nested, in-list), and source-level import boundary guards.

Verified: pnpm typecheck (6 workspaces), pnpm lint (0 errors),
packages/views vitest 240 files / 2743 tests all passing.

Co-authored-by: multica-agent 
Co-Authored-By: Claude Opus 4.8 
---
 .../components/chat-message-list.test.tsx     |  37 +-
 .../chat/components/chat-message-list.tsx     | 279 +++++++---
 packages/views/common/markdown.test.tsx       | 182 -------
 packages/views/common/markdown.tsx            | 140 -----
 packages/views/editor/attachment.tsx          |   4 +-
 packages/views/editor/readonly-content.tsx    | 437 +--------------
 packages/views/package.json                   |   1 +
 packages/views/rich-content/index.ts          |  12 +
 .../views/rich-content/rich-code-block.tsx    | 173 ++++++
 .../rich-content-boundary.test.ts             | 160 ++++++
 .../rich-content/rich-content-parity.test.tsx | 500 ++++++++++++++++++
 packages/views/rich-content/rich-content.css  |  37 ++
 packages/views/rich-content/rich-content.tsx  | 429 +++++++++++++++
 .../rich-content/streaming-fence.test.ts      | 102 ++++
 .../views/rich-content/streaming-fence.ts     | 105 ++++
 .../views/skills/components/file-viewer.tsx   |  10 +-
 pnpm-lock.yaml                                |  29 +-
 17 files changed, 1793 insertions(+), 844 deletions(-)
 delete mode 100644 packages/views/common/markdown.test.tsx
 delete mode 100644 packages/views/common/markdown.tsx
 create mode 100644 packages/views/rich-content/index.ts
 create mode 100644 packages/views/rich-content/rich-code-block.tsx
 create mode 100644 packages/views/rich-content/rich-content-boundary.test.ts
 create mode 100644 packages/views/rich-content/rich-content-parity.test.tsx
 create mode 100644 packages/views/rich-content/rich-content.css
 create mode 100644 packages/views/rich-content/rich-content.tsx
 create mode 100644 packages/views/rich-content/streaming-fence.test.ts
 create mode 100644 packages/views/rich-content/streaming-fence.ts

diff --git a/packages/views/chat/components/chat-message-list.test.tsx b/packages/views/chat/components/chat-message-list.test.tsx
index ef281515ad2..e5bbb42e9eb 100644
--- a/packages/views/chat/components/chat-message-list.test.tsx
+++ b/packages/views/chat/components/chat-message-list.test.tsx
@@ -1,11 +1,46 @@
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 import { act, render, screen } from "@testing-library/react";
 import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
 import { I18nProvider } from "@multica/core/i18n/react";
 import { chatKeys } from "@multica/core/chat/queries";
 import type { TaskMessagePayload } from "@multica/core/types";
+import type { ReactElement } from "react";
 import enChat from "../../locales/en/chat.json";
 
+// The live timeline is a real list row rather than Virtuoso chrome (MUL-4922),
+// so it shares one identity with the persisted assistant row and keeps its
+// Mermaid/HTML blocks mounted across task completion. Real react-virtuoso
+// renders its Footer but NO data rows under jsdom's zero-height viewport, so
+// these tests must stub it to see rows at all. computeItemKey is still applied
+// here — it is what expresses the live -> persisted identity.
+vi.mock("react-virtuoso", () => ({
+  Virtuoso: ({
+    data,
+    itemContent,
+    computeItemKey,
+    components,
+    context,
+  }: {
+    data: unknown[];
+    itemContent: (i: number, item: unknown) => ReactElement;
+    computeItemKey: (i: number, item: unknown) => string;
+    components?: { Footer?: (p: { context?: unknown }) => ReactElement | null };
+    context?: unknown;
+  }) => {
+    const Footer = components?.Footer;
+    return (
+      
+ {data.map((item, i) => ( +
+ {itemContent(i, item)} +
+ ))} + {Footer ?
: null} +
+ ); + }, +})); + import { ChatMessageList } from "./chat-message-list"; const TEST_RESOURCES = { en: { chat: enChat } }; diff --git a/packages/views/chat/components/chat-message-list.tsx b/packages/views/chat/components/chat-message-list.tsx index 1ed5f24aa0c..9e5d3d4c0c2 100644 --- a/packages/views/chat/components/chat-message-list.tsx +++ b/packages/views/chat/components/chat-message-list.tsx @@ -1,6 +1,6 @@ "use client"; -import { memo, useCallback, useMemo, useRef, useState } from "react"; +import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; import { toast } from "sonner"; import { useQuery } from "@tanstack/react-query"; import { Virtuoso, type Components } from "react-virtuoso"; @@ -20,7 +20,7 @@ import { import { ChevronRight, ChevronDown, Brain, AlertCircle, AlertTriangle, Copy } from "lucide-react"; import { useScrollFade } from "@multica/ui/hooks/use-scroll-fade"; import { isTaskMessageTaskId, taskMessagesOptions } from "@multica/core/chat/queries"; -import { MemoizedMarkdown } from "@multica/views/common/markdown"; +import { RichContent } from "../../rich-content"; import { copyText } from "@multica/ui/lib/clipboard"; import { AttachmentList } from "../../issues/components/comment-card"; import type { AgentAvailability } from "@multica/core/agents"; @@ -70,14 +70,34 @@ interface ChatMessageListProps { interface ChatListContext { isFetchingOlderMessages: boolean; - hasLive: boolean; - liveTimeline: ChatTimelineItem[]; showStatusPill: boolean; pendingTask: ChatPendingTask | null | undefined; liveTaskMessages: readonly TaskMessagePayload[] | undefined; availability: AgentAvailability | undefined; } +/** + * One Virtuoso row. A live (still-streaming) task and the persisted assistant + * message it becomes share ONE key — `task:` — so the handoff replaces + * this item's data in place instead of unmounting a Footer subtree and mounting + * a different row (MUL-4922). That identity is what keeps an already-rendered + * Mermaid diagram or HTML iframe mounted across task completion. + */ +type ChatRenderItem = + | { key: string; kind: "message"; message: ChatMessage; taskId: string | null } + | { key: string; kind: "live"; taskId: string }; + +/** + * Row key for a persisted message. Assistant turns carrying a task_id key on + * the task so they can inherit the live row; everything else keys on its own + * id. + */ +function messageRowKey(message: ChatMessage): string { + return message.role === "assistant" && message.task_id + ? `task:${message.task_id}` + : message.id; +} + function ChatListHeader({ context }: { context?: ChatListContext }) { const { t } = useT("chat"); return ( @@ -91,27 +111,24 @@ function ChatListHeader({ context }: { context?: ChatListContext }) { ); } +// The Footer now carries only the status pill — task chrome, not content. The +// live timeline moved into a real row so it can keep its identity when the +// task completes (see ChatRenderItem). function ChatListFooter({ context }: { context?: ChatListContext }) { if (!context) return null; + if (!context.showStatusPill || !context.pendingTask) return null; return (
- {context.hasLive && ( -
- -
- )} - {context.showStatusPill && context.pendingTask && ( - - )} +
); } -const LIST_COMPONENTS: Components = { +const LIST_COMPONENTS: Components = { Header: ChatListHeader, Footer: ChatListFooter, }; @@ -148,30 +165,39 @@ export function ChatMessageList({ ); // Live timeline for the in-flight task. useRealtimeSync keeps this cache - // current via setQueryData on task:message events. + // current via setQueryData on task:message events. Only used here to decide + // whether the live row exists and to feed the status pill — the row itself + // reads the same cache entry through AssistantMessage. const showLiveTimeline = !!pendingTaskId && !pendingAlreadyPersisted; const canFetchLiveTimeline = isTaskMessageTaskId(pendingTaskId) && !pendingAlreadyPersisted; const { data: liveTaskMessages } = useQuery({ ...taskMessagesOptions(pendingTaskId ?? ""), enabled: canFetchLiveTimeline, }); - // Memoized on the cache array identity: mergeTaskMessagesBySeq preserves - // the array reference when a duplicate event arrives, so this recomputes - // only when a genuinely new message lands — not on unrelated re-renders. - const liveTimeline: ChatTimelineItem[] = useMemo( - () => transformTimeline(buildTimeline(liveTaskMessages ?? []), transformContent), - [liveTaskMessages, transformContent], - ); - const hasLive = showLiveTimeline && liveTimeline.length > 0; + const hasLive = showLiveTimeline && (liveTaskMessages?.length ?? 0) > 0; const showStatusPill = !!pendingTaskId && !pendingAlreadyPersisted && !!pendingTask; - const totalCount = messages.length + (hasLive || showStatusPill ? 1 : 0); - const firstIndex = totalCount > 0 ? firstItemIndex : 0; + // Persisted messages plus, while a task is in flight, one synthetic trailing + // row for it. When the assistant message persists, `hasLive` goes false and + // the message takes the SAME key at the SAME position — an in-place data + // swap, not a remount. + const renderItems: ChatRenderItem[] = useMemo(() => { + const items: ChatRenderItem[] = messages.map((message) => ({ + key: messageRowKey(message), + kind: "message" as const, + message, + taskId: message.task_id ?? null, + })); + if (hasLive && pendingTaskId) { + items.push({ key: `task:${pendingTaskId}`, kind: "live", taskId: pendingTaskId }); + } + return items; + }, [messages, hasLive, pendingTaskId]); + + const firstIndex = renderItems.length > 0 ? firstItemIndex : 0; const listContext: ChatListContext = { isFetchingOlderMessages, - hasLive, - liveTimeline, showStatusPill, pendingTask, liveTaskMessages, @@ -192,7 +218,7 @@ export function ChatMessageList({ ) : ( msg.id} + computeItemKey={(_, item) => item.key} context={listContext} components={LIST_COMPONENTS} - itemContent={(_, msg) => ( + itemContent={(_, item) => (
@@ -266,25 +292,44 @@ export function ChatMessageSkeleton() { // memo skips reconciling rows the stream didn't touch — the persisted // history stays inert while only the live footer updates. const MessageBubble = memo(function MessageBubble({ - message, + item, isPending, transformContent, }: { - message: ChatMessage; + item: ChatRenderItem; isPending: boolean; transformContent?: (content: string) => string; }) { + // The live row and the persisted assistant row both land here under one key, + // and both render — same component type, same position — + // so React reconciles rather than remounts at task completion. + if (item.kind === "live") { + return ( + + ); + } + + const { message } = item; + if (message.role === "user") { return (
- {/* User messages are authored as markdown in ContentEditor, so - * render them through the same pipeline as assistant replies. - * Neutralise prose's leading/trailing margin so single-line - * bubbles stay as compact as the plain-text version used to. */} -
- {message.content} -
+ {/* User messages are authored as markdown in ContentEditor, so they + * render through the SAME RichContent as assistant replies and as + * Issue/Comment — a Mermaid fence a user pastes is a diagram here + * too. `compact` trims the leading/trailing block margins so a + * single-line bubble stays as tight as the plain-text version. */} + `), so the live → persisted + * handoff is a prop change rather than an unmount: the RichContent subtree and + * any Mermaid diagram / HTML iframe inside it stay mounted, keep their pan-zoom + * state, and never re-run their expensive render. Before this, the live + * timeline lived in Virtuoso's Footer and the persisted row keyed on + * `message.id`, so every completed task tore down and rebuilt its diagrams. + * + * The timeline itself comes from `taskMessagesOptions(taskId)` in both forms — + * the same cache entry useRealtimeSync seeds during execution — so no refetch + * and no data discontinuity happens at the handoff either. + */ function AssistantMessage({ + taskId, message, isPending, transformContent, }: { - message: ChatMessage; + taskId: string | null; + message?: ChatMessage; isPending: boolean; transformContent?: (content: string) => string; }) { - const taskId = message.task_id; const canFetchTaskMessages = isTaskMessageTaskId(taskId); // Use the shared taskMessagesOptions so this cache entry is the same one @@ -324,17 +388,23 @@ function AssistantMessage({ enabled: canFetchTaskMessages, }); - // Same memoization rationale as the live timeline in ChatMessageList. + // Memoized on the cache array identity: mergeTaskMessagesBySeq preserves the + // array reference when a duplicate event arrives, so this recomputes only + // when a genuinely new message lands. const timeline: ChatTimelineItem[] = useMemo( () => transformTimeline(buildTimeline(taskMessages ?? []), transformContent), [taskMessages, transformContent], ); + // Content is settled once the persisted message exists; until then text is + // still arriving and a trailing fence may be half-written. + const phase: "streaming" | "settled" = message ? "settled" : "streaming"; + // Failure bubble path: when the server's FailTask wrote a failure // chat_message (failure_reason set), render a destructive bubble with the // human-readable reason label + collapsible raw errMsg + the same timeline // so the user can see exactly where the run broke. - if (message.failure_reason) { + if (message?.failure_reason) { return ( {timeline.length > 0 && ( - + )} {isNoResponse ? ( - ) : timeline.length === 0 ? ( -
- {message.content} -
+ ) : message && timeline.length === 0 ? ( + ) : null} - - + {message && ( + <> + + + + )}
); } @@ -584,35 +667,42 @@ function TimelineView({ items, isStreaming, attachments, + phase = "settled", }: { items: ChatTimelineItem[]; isStreaming?: boolean; attachments?: import("@multica/core/types").Attachment[]; + phase?: "streaming" | "settled"; }) { const { preface, middle, final } = splitTimeline(items); return ( <> {preface.length > 0 && ( -
- - {preface.map((t) => t.content ?? "").join("")} - -
+ t.content ?? "").join("")} + attachments={attachments} + density="compact" + phase={phase} + className="leading-relaxed" + /> )} {middle.length > 0 && ( )} {final.length > 0 && ( -
- - {final.map((t) => t.content ?? "").join("")} - -
+ t.content ?? "").join("")} + attachments={attachments} + density="compact" + phase={phase} + className="leading-relaxed" + /> )} ); @@ -620,20 +710,27 @@ function TimelineView({ function OuterProcessFold({ items, - defaultOpen, + isStreaming, attachments, + phase = "settled", }: { items: ChatTimelineItem[]; - defaultOpen?: boolean; + isStreaming?: boolean; attachments?: import("@multica/core/types").Attachment[]; + phase?: "streaming" | "settled"; }) { const { t } = useT("chat"); - // useState seeds once at mount — subsequent renders never overwrite the - // user's manual toggle. The streaming → completed transition unmounts - // the live and mounts the persisted AssistantMessage's - // own , so the persisted instance starts closed (default) - // even if the live one was open. That's the desired collapsed-default. - const [open, setOpen] = useState(defaultOpen ?? false); + // Open while the task streams (so the user watches progress), collapsed once + // it settles. This used to fall out of a remount: the live TimelineView was + // torn down and the persisted one mounted closed. The row is now stable + // across that handoff (MUL-4922) — which is the point, it keeps Mermaid and + // HTML blocks alive — so the collapse has to be expressed directly. + const [open, setOpen] = useState(!!isStreaming); + const wasStreaming = useRef(!!isStreaming); + useEffect(() => { + if (wasStreaming.current && !isStreaming) setOpen(false); + wasStreaming.current = !!isStreaming; + }, [isStreaming]); const stepCount = items.length; return ( @@ -646,7 +743,12 @@ function OuterProcessFold({
{items.map((item) => item.type === "text" ? ( - + ) : ( ), @@ -664,13 +766,20 @@ function OuterProcessFold({ function MiddleTextRow({ item, attachments, + phase = "settled", }: { item: ChatTimelineItem; attachments?: import("@multica/core/types").Attachment[]; + phase?: "streaming" | "settled"; }) { return ( -
- {item.content ?? ""} +
+
); } diff --git a/packages/views/common/markdown.test.tsx b/packages/views/common/markdown.test.tsx deleted file mode 100644 index 0e3450e33f9..00000000000 --- a/packages/views/common/markdown.test.tsx +++ /dev/null @@ -1,182 +0,0 @@ -import type { ReactNode } from "react"; -import { render, screen } from "@testing-library/react"; -import { afterEach, describe, expect, it, vi } from "vitest"; -import { Markdown as MarkdownBase } from "@multica/ui/markdown"; -import { Markdown } from "./markdown"; - -const { resolveMock } = vi.hoisted(() => ({ resolveMock: vi.fn() })); - -vi.mock("../issues/hooks", () => ({ - useResolveIssueIdentifier: (identifier: string) => resolveMock(identifier), -})); - -vi.mock("@multica/core/config", () => ({ - useConfigStore: (selector: (state: { cdnDomain: string }) => unknown) => - selector({ cdnDomain: "" }), -})); - -vi.mock("../issues/components/issue-mention-card", () => ({ - IssueMentionCard: ({ issueId }: { issueId: string }) => ( - {issueId} - ), -})); - -vi.mock("@multica/core/paths", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - useWorkspaceSlug: () => "acme", - useRequiredWorkspaceSlug: () => "acme", - useWorkspacePaths: () => ({ - ...actual.paths.workspace("acme"), - projectDetail: (projectId: string) => `/projects/${projectId}`, - }), - }; -}); - -vi.mock("../navigation", () => ({ - AppLink: ({ - href, - children, - className, - }: { - href: string; - children: ReactNode; - className?: string; - }) => ( - - {children} - - ), -})); - -vi.mock("../projects/components/project-chip", () => ({ - ProjectChip: ({ projectId }: { projectId: string }) => ( - {projectId} - ), -})); - -const ligatureClasses = [ - "[font-variant-ligatures:none]", - "[font-feature-settings:'liga'_0]", -]; - -describe("Markdown", () => { - it("disables ligatures inside raw code tags", () => { - render({"uv run --extra dev pytest -q"}); - - expect(screen.getByText("uv run --extra dev pytest -q")).toHaveClass(...ligatureClasses); - }); - - it("disables ligatures inside fenced code blocks", () => { - render({"```sh\nuv run --extra dev pytest -q\n```"}); - - expect(screen.getByText("uv run --extra dev pytest -q")).toHaveClass(...ligatureClasses); - }); - - it("disables ligatures in terminal-mode code", () => { - render({"uv run --extra dev pytest -q"}); - - expect(screen.getByText("uv run --extra dev pytest -q")).toHaveClass(...ligatureClasses); - }); - - it("renders slash skill links as slash command pills", () => { - const { container } = render( - [/deploy](slash://skill/abc-123), - ); - - const pill = container.querySelector(".slash-command"); - expect(pill).not.toBeNull(); - expect(pill?.textContent).toBe("/deploy"); - }); - - it("renders project mention links as project chips", () => { - render({"[Roadmap](mention://project/project-123)"}); - - expect(screen.getByTestId("project-chip")).toHaveTextContent("project-123"); - expect(screen.getByRole("link")).toHaveAttribute("href", "/projects/project-123"); - }); -}); - -describe("Markdown bare issue identifier autolink", () => { - afterEach(() => { - resolveMock.mockReset(); - }); - - it("renders a resolved bare identifier as an issue chip", () => { - resolveMock.mockImplementation((id: string) => - id === "MUL-1" ? { id: "issue-1", identifier: "MUL-1" } : null, - ); - - render({"Related to MUL-1 today"}); - - expect(screen.getByTestId("issue-mention-card")).toHaveTextContent("issue-1"); - expect(resolveMock).toHaveBeenCalledWith("MUL-1"); - }); - - it("leaves an unresolved bare identifier as plain text", () => { - resolveMock.mockReturnValue(null); - - render({"Related to MUL-999 today"}); - - expect(screen.queryByTestId("issue-mention-card")).toBeNull(); - expect(screen.getByText(/Related to/)).toHaveTextContent("MUL-999"); - }); - - it("does not autolink identifiers inside inline code", () => { - resolveMock.mockReturnValue(null); - - render({"use `MUL-1` here"}); - - expect(resolveMock).not.toHaveBeenCalled(); - expect(screen.queryByTestId("issue-mention-card")).toBeNull(); - }); - - it("routes a canonical UUID mention straight to the chip (no identifier resolve)", () => { - render( - - {"[MUL-2](mention://issue/00000000-0000-0000-0000-000000000002)"} - , - ); - - expect(screen.getByTestId("issue-mention-card")).toHaveTextContent( - "00000000-0000-0000-0000-000000000002", - ); - expect(resolveMock).not.toHaveBeenCalled(); - }); -}); - -// The base renderer uses a plain ; exercising it here (instead of the -// views wrapper, which swaps in ) lets us assert the sanitized -// `src` directly. Covers the two gates that used to blank data-URI images: -// rehype-sanitize's protocols.src and react-markdown's urlTransform. -describe("Markdown inline data-URI images", () => { - const PNG_1X1 = - "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg=="; - - it("preserves the src of an inline data:image/png image", () => { - render({`![demo](${PNG_1X1})`}); - - const img = screen.getByAltText("demo"); - expect(img.tagName).toBe("IMG"); - expect(img).toHaveAttribute("src", PNG_1X1); - }); - - it("keeps regular http(s) images working", () => { - render({"![cat](https://cdn.example.com/cat.png)"}); - - expect(screen.getByAltText("cat")).toHaveAttribute( - "src", - "https://cdn.example.com/cat.png", - ); - }); - - it("strips non-image data URIs (data:text/html)", () => { - render( - {"![x](data:text/html,)"}, - ); - - const img = screen.getByAltText("x"); - expect(img.getAttribute("src") ?? "").toBe(""); - }); -}); diff --git a/packages/views/common/markdown.tsx b/packages/views/common/markdown.tsx deleted file mode 100644 index 920efda33c1..00000000000 --- a/packages/views/common/markdown.tsx +++ /dev/null @@ -1,140 +0,0 @@ -"use client"; - -import * as React from "react"; -import { - Markdown as MarkdownBase, - type MarkdownProps as MarkdownBaseProps, - type RenderMode, - isIssueIdentifier, -} from "@multica/ui/markdown"; -import { useConfigStore } from "@multica/core/config"; -import type { Attachment as AttachmentRecord } from "@multica/core/types"; -import { useWorkspacePaths } from "@multica/core/paths"; -import { IssueMentionCard } from "../issues/components/issue-mention-card"; -import { useResolveIssueIdentifier } from "../issues/hooks"; -import { ProjectChip } from "../projects/components/project-chip"; -import { AppLink } from "../navigation"; -import { - Attachment as AttachmentRenderer, - AttachmentDownloadProvider, -} from "../editor"; - -export type { RenderMode }; - -export interface MarkdownProps extends MarkdownBaseProps { - /** - * Attachments associated with the surrounding entity (chat message, skill - * file). When passed, the renderer resolves inline image / file-card URLs - * to full attachment records via AttachmentDownloadProvider, unlocking the - * unified hover toolbar / lightbox / preview-modal behavior used in - * editor surfaces. - */ - attachments?: AttachmentRecord[]; -} - -/** - * Default renderMention that delegates to entity chips for issue/project mentions - * and renders a styled span for other mention types. - */ -function ProjectMentionCard({ projectId }: { projectId: string }): React.ReactNode { - const p = useWorkspacePaths(); - return ( - - - - ); -} - -/** - * Autolinked bare identifier (e.g. `MUL-123`) routed through - * `mention://issue/`. Resolves the identifier to a real issue in - * the current workspace; renders a navigable chip on a hit, plain text on a - * miss / while loading / cross-workspace. - */ -function AutolinkedIssueMention({ identifier }: { identifier: string }): React.ReactNode { - const issue = useResolveIssueIdentifier(identifier); - if (!issue) return identifier; - return ; -} - -function defaultRenderMention({ - type, - id, -}: { - type: string; - id: string; -}): React.ReactNode { - if (type === "issue") { - // A bare identifier (from the autolink preprocessor) is carried as the id - // segment; a real mention carries a UUID. Dispatch on the id shape. - if (isIssueIdentifier(id)) { - return ; - } - return ; - } - if (type === "project") { - return ; - } - return null; -} - -function renderImage({ src, alt }: { src: string; alt: string }): React.ReactNode { - return ( - - ); -} - -function renderFileCard({ - href, - filename, -}: { - href: string; - filename: string; -}): React.ReactNode { - return ( - - ); -} - -/** - * App-level Markdown wrapper. Injects: - * - entity chips for issue/project mentions - * - cdnDomain from the config store (drives fileCard preprocessing) - * - unified as the image / file-card renderer - * - AttachmentDownloadProvider so url → record resolution works inside - * the injected components - */ -export function Markdown(props: MarkdownProps): React.JSX.Element { - const cdnDomain = useConfigStore((s) => s.cdnDomain); - const { attachments, ...rest } = props; - return ( - - - - ); -} - -export const MemoizedMarkdown = React.memo(Markdown); -MemoizedMarkdown.displayName = "MemoizedMarkdown"; diff --git a/packages/views/editor/attachment.tsx b/packages/views/editor/attachment.tsx index c85d9abb2cc..534579d0575 100644 --- a/packages/views/editor/attachment.tsx +++ b/packages/views/editor/attachment.tsx @@ -14,9 +14,9 @@ * Call sites: * - extensions/file-card.tsx FileCardView (Tiptap NodeView) * - extensions/image-view.tsx ImageView (Tiptap NodeView) - * - readonly-content.tsx (markdown img + fileCard div renderers) + * - rich-content/rich-content.tsx (markdown img + fileCard div renderers, + * serving Chat, Issue descriptions and Comments through one renderer) * - issues/components/comment-card.tsx AttachmentList (standalone fallback) - * - common/markdown.tsx (chat / skill viewer Markdown wrapper) * * The component owns its own preview modal and download dispatcher — callers * just pass `attachment` and (for editor surfaces) optional editor chrome diff --git a/packages/views/editor/readonly-content.tsx b/packages/views/editor/readonly-content.tsx index e7aa2f82128..0fb60c7af0e 100644 --- a/packages/views/editor/readonly-content.tsx +++ b/packages/views/editor/readonly-content.tsx @@ -1,441 +1,46 @@ "use client"; /** - * ReadonlyContent — lightweight markdown renderer for readonly content display. + * ReadonlyContent — compatibility wrapper over the canonical . * - * Replaces for comment cards and other - * read-only surfaces. Uses react-markdown instead of a full Tiptap/ProseMirror - * instance, eliminating EditorView, Plugin, and NodeView overhead. + * The renderer itself moved to packages/views/rich-content/ (MUL-4922) so Chat, + * Issue descriptions and Comments all share ONE implementation of Markdown + * parsing, sanitize, fenced-code dispatch, mentions, links and attachments. + * This file stays only so existing document-density callers (comment cards, + * issue detail, autopilot detail, Markdown attachment preview) keep their + * import path and props. * - * Visual parity with ContentEditor is achieved by: - * - Wrapping output in
so the same - * styles/index.css rules apply to standard HTML tags - * - Using the same preprocessMarkdown pipeline (mention shortcodes + linkify) - * - Using lowlight for code highlighting (same engine as Tiptap's CodeBlockLowlight) - * so .hljs-* CSS rules from styles/code.css produce identical colors - * - Rendering mentions with the same IssueMentionCard component and .mention class + * Do not reintroduce rendering logic here. New behaviour belongs in + * RichContent, where every surface picks it up at once. */ -import { isValidElement, memo, useMemo, useRef, useState } from "react"; -import ReactMarkdown, { type Components } from "react-markdown"; -import type { ReactNode } from "react"; -import rehypeKatex from "rehype-katex"; -import remarkBreaks from "remark-breaks"; -import remarkGfm from "remark-gfm"; -import remarkMath from "remark-math"; -import rehypeRaw from "rehype-raw"; -import rehypeSanitize from "rehype-sanitize"; -import { toHtml } from "hast-util-to-html"; -import { Check, Copy } from "lucide-react"; -import { cn } from "@multica/ui/lib/utils"; -import { copyText } from "@multica/ui/lib/clipboard"; -import { useWorkspacePaths, useWorkspaceSlug } from "@multica/core/paths"; +import { memo } from "react"; import type { Attachment } from "@multica/core/types"; -import { useT } from "../i18n"; -import { useNavigation } from "../navigation"; -import { IssueMentionCard } from "../issues/components/issue-mention-card"; -import { useResolveIssueIdentifier } from "../issues/hooks"; -import { ProjectChip } from "../projects/components/project-chip"; -import { useLinkHover, LinkHoverCard } from "./link-hover-card"; -import { openLink, isMentionHref } from "./utils/link-handler"; -import { - isAllowedFileCardHref, - isIssueIdentifier, - markdownSanitizeSchema, - markdownUrlTransform, -} from "@multica/ui/markdown"; -import { preprocessMarkdown } from "./utils/preprocess"; -import { highlightToHtml } from "./utils/highlight-markdown"; -import { MermaidDiagram } from "./mermaid-diagram"; -import { HtmlBlockPreview } from "./html-block-preview"; -import { AttachmentDownloadProvider } from "./attachment-download-context"; -import { Attachment as AttachmentRenderer } from "./attachment"; -import { highlightCode } from "./syntax-highlight"; -import "katex/dist/katex.min.css"; -import "./styles/index.css"; - -// Code fences that the `code` renderer returns as a non- React element -// (Mermaid diagram, HTML preview iframe). The `pre` renderer below unwraps -// these so the default
 envelope doesn't clamp their styles.
-// Anchored to whole class tokens so `language-htmlbars` / `language-mermaidx`
-// don't accidentally match and lose their 
 wrapper.
-const PRE_UNWRAP_RE = /(^|\s)language-(html|mermaid)(\s|$)/;
-
-// ---------------------------------------------------------------------------
-// Custom react-markdown components
-// ---------------------------------------------------------------------------
-
-/**
- * Issue mention chip. Navigation — plain click, modifier click, and the
- * "open issue links in new tab" preference — is owned by the AppLink inside
- * IssueMentionCard; the wrapper only shields surrounding click handlers
- * (e.g. collapsed-comment expanders) from mention clicks.
- */
-function IssueMentionLink({ issueId, label }: { issueId: string; label?: string }) {
-  return (
-     e.stopPropagation()}>
-      
-    
-  );
-}
-
-/**
- * Autolinked bare identifier (e.g. `MUL-123`) routed through
- * `mention://issue/` by the readonly preprocessor. Resolves to a
- * real issue in the current workspace; renders a navigable mention on a hit,
- * plain text on a miss / while loading / cross-workspace.
- */
-function AutolinkedIssueMentionLink({ identifier }: { identifier: string }) {
-  const issue = useResolveIssueIdentifier(identifier);
-  if (!issue) return <>{identifier};
-  return ;
-}
-
-function ProjectMentionLink({ projectId, label }: { projectId: string; label?: string }) {
-  const { push, openInNewTab } = useNavigation();
-  const p = useWorkspacePaths();
-  const path = p.projectDetail(projectId);
-  return (
-     {
-        e.preventDefault();
-        e.stopPropagation();
-        if (e.metaKey || e.ctrlKey || e.shiftKey) {
-          if (openInNewTab) {
-            openInNewTab(path, label);
-          }
-          return;
-        }
-        push(path);
-      }}
-    >
-      
-    
-  );
-}
-
-function getTextContent(node: ReactNode): string {
-  if (node == null || typeof node === "boolean") return "";
-  if (typeof node === "string" || typeof node === "number") return String(node);
-  if (Array.isArray(node)) return node.map(getTextContent).join("");
-  if (isValidElement(node)) {
-    const props = node.props as { children?: ReactNode };
-    return getTextContent(props.children);
-  }
-  return "";
-}
-
-function ReadonlyCodeBlock({
-  children,
-  language,
-}: {
-  children: ReactNode;
-  language?: string;
-}) {
-  const { t } = useT("editor");
-  const [copied, setCopied] = useState(false);
-  const code = useMemo(
-    () => getTextContent(children).replace(/\n$/, ""),
-    [children],
-  );
-  const copyLabel = t(($) => $.code_block.copy_code) || "Copy code";
-
-  const handleCopy = async () => {
-    if (!code) return;
-    if (await copyText(code)) {
-      setCopied(true);
-      setTimeout(() => setCopied(false), 2000);
-    }
-  };
-
-  return (
-    
-
- {/* Same hover chrome as the editable code block's header - (code-block-view.tsx): language label + copy. */} - {language && ( - - {language} - - )} - -
- {/* No extra right padding: `.rich-text-editor pre` outranks utility - padding classes anyway, and the editable NodeView uses the same - 1rem — keeping them identical keeps line wrapping identical. */} -
{children}
-
- ); -} - -// Named component so it can call useWorkspaceSlug() — arrow function inlined -// inside `components` below would still work, but extracting it keeps the -// hook usage explicit and avoids hook-in-object-literal surprises. -function ReadonlyLink({ - href, - children, -}: { - href?: string; - children?: React.ReactNode; -}) { - const slug = useWorkspaceSlug(); - - if (href?.startsWith("slash://skill/")) { - return {children}; - } - - if (isMentionHref(href)) { - const match = href.match(/^mention:\/\/(member|agent|issue|project|all)\/(.+)$/); - if (match?.[1] === "issue" && match[2]) { - // A bare identifier (from the autolink preprocessor) is carried as the id - // segment; a real mention carries a UUID. Dispatch on the id shape. - if (isIssueIdentifier(match[2])) { - return ; - } - const label = - typeof children === "string" - ? children - : Array.isArray(children) - ? children.join("") - : undefined; - return ; - } - if (match?.[1] === "project" && match[2]) { - const label = - typeof children === "string" - ? children - : Array.isArray(children) - ? children.join("") - : undefined; - return ; - } - // Member / agent / all mentions - return {children}; - } - - // Regular links — open directly on click - return ( - { - e.preventDefault(); - if (href) openLink(href, slug); - }} - > - {children} - - ); -} - -function buildComponents(): Partial { - return { - // Links — route mention:// to mention components, others show preview card - a: ReadonlyLink, - - // Images — unified through . The resolver context provided - // by AttachmentDownloadProvider (mounted in ReadonlyContent below) turns - // a CDN URL into a full record when possible; external URLs render as - // plain images with lightbox-via-preview-modal. forceKind is mandatory - // here because markdown `![]()` carries no content-type and alt is - // commonly empty or descriptive — without it images fall through to - // the file-card chrome. - img: ({ src, alt }) => ( - - ), - - // FileCard — intercept
from preprocessMarkdown - div: ({ node, children, ...props }) => { - const dataType = node?.properties?.dataType as string | undefined; - if (dataType === "fileCard") { - const rawHref = (node?.properties?.dataHref as string) || ""; - const href = isAllowedFileCardHref(rawHref) ? rawHref : ""; - const filename = (node?.properties?.dataFilename as string) || ""; - return ( - - ); - } - return
{children}
; - }, - - // Tables — wrap in tableWrapper div for border/radius/scroll (matches Tiptap) - table: ({ children }) => ( -
- {children}
-
- ), - - // Code — lowlight highlighting for blocks, plain render for inline - code: ({ className, children, node, ...props }) => { - const lang = /language-(\w+)/.exec(className || "")?.[1]; - const isBlock = - node?.position && - node.position.start.line !== node.position.end.line; - - if (isBlock && lang === "mermaid") { - return ; - } - if (isBlock && lang === "html") { - // Like Mermaid, return the React element directly here and rely on - // the `pre` renderer below to unwrap it — react-markdown otherwise - // wraps `code` children in a `
` whose monospace + overflow
-        // styles would clamp the preview iframe.
-        return ;
-      }
-
-      if (!isBlock && !lang) {
-        // Inline code — CSS handles styling via .rich-text-editor code
-        return {children};
-      }
-
-      // Block code — highlight with lowlight, output hljs classes
-      const code = String(children).replace(/\n$/, "");
-      try {
-        const tree = highlightCode(code, lang);
-        const html = toHtml(tree);
-        if (html) {
-          return (
-            
-          );
-        }
-      } catch {
-        // fall through to plain render
-      }
-      return (
-        
-          {children}
-        
-      );
-    },
-
-    // Pre — wrap regular code fences with copy chrome.
-    // Special-case Mermaid / HtmlBlockPreview returned from the `code`
-    // renderer above so the outer `
` does not wrap them — this is the
-    // standard two-layer pattern used to escape react-markdown's default
-    // `
` envelope.
-    pre: ({ children }) => {
-      // react-markdown calls `pre` BEFORE invoking the `code` renderer —
-      // `children` is the unrendered `` element from the AST. So we
-      // identify "this block was meant to be unwrapped" by inspecting the
-      // child's className (`language-mermaid`, `language-html`), not by
-      // checking `children.type === MermaidDiagram`, which never matches.
-      //
-      // Match by exact class token: a substring `includes("language-html")`
-      // would also fire on neighboring languages like `language-htmlbars`
-      // and silently strip their 
 wrapper.
-      let language: string | undefined;
-      if (isValidElement(children)) {
-        const childProps = children.props as { className?: string };
-        if (PRE_UNWRAP_RE.test(childProps.className ?? "")) {
-          return <>{children};
-        }
-        language = /language-(\w+)/.exec(childProps.className ?? "")?.[1];
-      }
-      return {children};
-    },
-  };
-}
-
-// ---------------------------------------------------------------------------
-// Component
-// ---------------------------------------------------------------------------
+import { RichContent } from "../rich-content";
 
 interface ReadonlyContentProps {
   content: string;
   className?: string;
   /**
-   * Attachments associated with the surrounding entity (comment / issue
-   * body). When the markdown contains an inline `` or file card whose
-   * URL matches one of these attachments, the download button re-signs the
-   * URL at click time via `useDownloadAttachment` instead of opening the
-   * potentially stale link embedded in the markdown.
-   *
-   * Callers SHOULD pass a stable reference (e.g. the field on a memoized
-   * timeline entry); a fresh array on every parent render busts the memo.
+   * Attachments associated with the surrounding entity (comment / issue body).
+   * Callers SHOULD pass a stable reference; a fresh array on every parent
+   * render busts the memo.
    */
   attachments?: Attachment[];
 }
 
-// Memoized so a long timeline of comments (Inbox + IssueDetail) does not
-// re-run the full react-markdown + rehype-* + lowlight pipeline on every
-// parent re-render. Props are `content`/`className`/`attachments`, all
-// shallow-comparable; stability is the caller's responsibility for the
-// array.
 export const ReadonlyContent = memo(function ReadonlyContent({
   content,
   className,
   attachments,
 }: ReadonlyContentProps) {
-  const processed = useMemo(
-    () =>
-      highlightToHtml(
-        preprocessMarkdown(content, { autolinkIssueIdentifiers: true }),
-      ),
-    [content],
-  );
-  const wrapperRef = useRef(null);
-  const hover = useLinkHover(wrapperRef);
-
-  // Components map is now static — all attachment-aware logic lives in
-  // , which reads the surrounding AttachmentDownloadProvider.
-  const components = useMemo(() => buildComponents(), []);
-
-  // Memoize the whole react-markdown subtree on its only real inputs
-  // (`processed` + `components`). Unrelated parent re-renders (e.g. a sibling
-  // agent task streaming over WebSocket fires one every ~100ms) would otherwise
-  // re-run react-markdown, which hands `` a fresh `dangerouslySetInnerHTML`
-  // object each time; React then rewrites the highlighted innerHTML even though
-  // the HTML string is byte-identical, tearing down and rebuilding every hljs
-  //  — which collapses any active text selection inside a code block
-  // (MUL-3621). A stable element reference lets React bail out of the subtree.
-  const markdown = useMemo(
-    () => (
-      
-        {processed}
-      
-    ),
-    [processed, components],
-  );
-
   return (
-    
-      
- {markdown} - -
-
+ ); }); diff --git a/packages/views/package.json b/packages/views/package.json index 9855631213b..5a1f167341c 100644 --- a/packages/views/package.json +++ b/packages/views/package.json @@ -89,6 +89,7 @@ "hast-util-to-html": "^9.0.5", "katex": "catalog:", "lowlight": "^3.3.0", + "mdast-util-from-markdown": "^2.0.3", "mermaid": "catalog:", "motion": "^12.38.0", "pinyin-pro": "3.26.0", diff --git a/packages/views/rich-content/index.ts b/packages/views/rich-content/index.ts new file mode 100644 index 00000000000..b1bbd9eca00 --- /dev/null +++ b/packages/views/rich-content/index.ts @@ -0,0 +1,12 @@ +export { + RichContent, + type RichContentProps, + type RichContentDensity, + type RichContentPhase, +} from "./rich-content"; +export { + isRichFenceLanguage, + shouldUpgradeFence, + type RichFenceLanguage, +} from "./rich-code-block"; +export { computeClosedFenceOffsets } from "./streaming-fence"; diff --git a/packages/views/rich-content/rich-code-block.tsx b/packages/views/rich-content/rich-code-block.tsx new file mode 100644 index 00000000000..8a1f43177c7 --- /dev/null +++ b/packages/views/rich-content/rich-code-block.tsx @@ -0,0 +1,173 @@ +"use client"; + +/** + * RichCodeBlock — the ONLY fenced-code dispatcher in the product (MUL-4922). + * + * Every product surface (Chat, Issue description, Comment) reaches fenced code + * through this file. Adding a language branch anywhere else — a per-surface + * `if (lang === "…")` in a message list or a comment card — is exactly the + * drift this sweep deleted, so new languages get added HERE and nowhere else. + * + * Dispatch is on a whole language token, never a substring: `language-htmlbars` + * and `language-mermaidx` are ordinary code, not an HTML preview / diagram. + * + * Upgrading to a rich block additionally requires the fence to be CLOSED (see + * streaming-fence.ts). A half-streamed fence renders as plain source, so + * Mermaid never parses a partial diagram and no iframe is created for HTML that + * is still arriving. + * + * Leaf components (MermaidDiagram / HtmlBlockPreview / lowlight static code) + * are surface-agnostic and shared with the Tiptap editor's NodeViews. They are + * imported by direct path — never through the `editor` barrel — so this module + * does not pull the editor's Tiptap graph into Chat. + */ + +import { memo, useMemo, useState, type ReactNode } from "react"; +import { toHtml } from "hast-util-to-html"; +import { Check, Copy } from "lucide-react"; +import { cn } from "@multica/ui/lib/utils"; +import { copyText } from "@multica/ui/lib/clipboard"; +import { useT } from "../i18n"; +import { MermaidDiagram } from "../editor/mermaid-diagram"; +import { HtmlBlockPreview } from "../editor/html-block-preview"; +import { highlightCode } from "../editor/syntax-highlight"; + +/** + * Languages that may become a rich block. Anything else — including unknown + * and absent languages — renders as static highlighted code. + */ +export type RichFenceLanguage = "mermaid" | "html"; + +export function isRichFenceLanguage( + language: string | undefined, +): language is RichFenceLanguage { + return language === "mermaid" || language === "html"; +} + +/** + * Whether a fenced block should render as a rich block rather than source. + * Both conditions are required: a rich-capable language AND a closed fence. + */ +export function shouldUpgradeFence( + language: string | undefined, + isFenceClosed: boolean, +): boolean { + return isRichFenceLanguage(language) && isFenceClosed; +} + +// Memoized on source so appending text elsewhere in a streaming message does +// not re-run Mermaid's async render or reload an already-mounted iframe. React +// reconciliation keeps the instance mounted (same element type, same position); +// memo additionally keeps it from re-rendering. +const MemoMermaidDiagram = memo(MermaidDiagram); +const MemoHtmlBlockPreview = memo(HtmlBlockPreview); + +/** + * Static lowlight-highlighted ``, matching the editable Tiptap code + * block's engine and `.hljs-*` CSS so a fence looks identical in every surface. + */ +export function StaticCodeBody({ + language, + body, + className, +}: { + language: string | undefined; + body: string; + className?: string; +}) { + const html = useMemo(() => { + const code = body.replace(/\n$/, ""); + try { + const tree = highlightCode(code, language); + return toHtml(tree) as string; + } catch { + return null; + } + }, [body, language]); + + if (html == null) { + // Highlighter failure must not blank the code — render it unhighlighted. + return ( + {body.replace(/\n$/, "")} + ); + } + + return ( + + ); +} + +/** + * `
` shell with hover copy chrome, matching the editable code block's
+ * header (language label + copy) in code-block-view.tsx.
+ */
+export function CodeBlockShell({
+  language,
+  code,
+  children,
+}: {
+  language?: string;
+  code: string;
+  children: ReactNode;
+}) {
+  const { t } = useT("editor");
+  const [copied, setCopied] = useState(false);
+  const copyLabel = t(($) => $.code_block.copy_code) || "Copy code";
+
+  const handleCopy = async () => {
+    if (!code) return;
+    if (await copyText(code)) {
+      setCopied(true);
+      setTimeout(() => setCopied(false), 2000);
+    }
+  };
+
+  return (
+    
+
+ {language && ( + + {language} + + )} + +
+ {/* No extra right padding: `.rich-text-editor pre` outranks utility + padding classes anyway, and the editable NodeView uses the same + 1rem — keeping them identical keeps line wrapping identical. */} +
{children}
+
+ ); +} + +/** + * The rich leaf for an upgraded fence. Only reached when shouldUpgradeFence() + * returned true, so the fence is known-closed here. + */ +export function RichFenceBlock({ + language, + body, +}: { + language: RichFenceLanguage; + body: string; +}) { + if (language === "mermaid") { + return ; + } + return ; +} diff --git a/packages/views/rich-content/rich-content-boundary.test.ts b/packages/views/rich-content/rich-content-boundary.test.ts new file mode 100644 index 00000000000..eded2eb93a0 --- /dev/null +++ b/packages/views/rich-content/rich-content-boundary.test.ts @@ -0,0 +1,160 @@ +/** + * Import boundary guard (MUL-4922, Howard's contract #3). + * + * The point of this sweep is that there is exactly ONE product-level readonly + * renderer. That property is not self-enforcing: the cheapest way to add a + * feature to Chat will always look like "render this bit with the generic + * Markdown component" or "pass a custom code renderer here", and each of those + * quietly recreates the second chain we just deleted. + * + * These tests read the actual source tree, so they fail on the commit that + * introduces the fork rather than months later when the surfaces have visibly + * diverged again. + */ +import { describe, expect, it } from "vitest"; +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join, relative } from "node:path"; + +const VIEWS_ROOT = join(__dirname, ".."); + +// Product surfaces that render user/agent-authored content. Any of these +// reaching for a generic Markdown renderer is the regression. +const PRODUCT_SURFACES = ["chat", "issues", "skills", "autopilots", "inbox"]; + +function walk(dir: string, out: string[] = []): string[] { + let entries: string[]; + try { + entries = readdirSync(dir); + } catch { + return out; + } + for (const entry of entries) { + if (entry === "node_modules" || entry.startsWith(".")) continue; + const full = join(dir, entry); + if (statSync(full).isDirectory()) walk(full, out); + else if (/\.tsx?$/.test(full) && !/\.test\.tsx?$/.test(full)) out.push(full); + } + return out; +} + +/** + * Drop comments before matching. These guards look for real imports and real + * language branches; prose that merely *mentions* one (a doc comment + * explaining which renderer mounts a leaf) is not a violation, and treating it + * as one would train the next person to delete the guard. + */ +function stripComments(text: string): string { + return text.replace(/\/\*[\s\S]*?\*\//g, "").replace(/(^|[^:])\/\/.*$/gm, "$1"); +} + +function sourceFiles(subdirs: string[]): { path: string; text: string }[] { + return subdirs + .flatMap((d) => walk(join(VIEWS_ROOT, d))) + .map((path) => ({ + path: relative(VIEWS_ROOT, path), + text: stripComments(readFileSync(path, "utf8")), + })); +} + +describe("RichContent import boundary", () => { + it("no product surface imports the generic ui Markdown renderer", () => { + const offenders = sourceFiles(PRODUCT_SURFACES) + .filter(({ text }) => + /from\s+["']@multica\/ui\/markdown["']/.test(text) && + /\bMarkdown\b|\bMemoizedMarkdown\b|\bStreamingMarkdown\b/.test(text), + ) + .map(({ path }) => path); + + expect(offenders).toEqual([]); + }); + + it("the deleted chat markdown bridge has not come back", () => { + const offenders = sourceFiles([...PRODUCT_SURFACES, "common", "editor"]) + .filter(({ text }) => /common\/markdown/.test(text)) + .map(({ path }) => path); + + expect(offenders).toEqual([]); + }); + + it("no product surface builds its own react-markdown pipeline", () => { + // react-markdown may be imported only by the canonical renderer. A second + // import means a second pipeline with its own sanitize + components map. + const offenders = sourceFiles([...PRODUCT_SURFACES, "common", "editor"]) + .filter(({ text }) => /from\s+["']react-markdown["']/.test(text)) + .map(({ path }) => path); + + expect(offenders).toEqual([]); + }); + + it("only the canonical renderer configures the sanitize schema", () => { + const offenders = sourceFiles([...PRODUCT_SURFACES, "common", "editor"]) + .filter(({ text }) => /rehype-sanitize|markdownSanitizeSchema/.test(text)) + .map(({ path }) => path); + + expect(offenders).toEqual([]); + }); + + it("only rich-code-block dispatches on a fence language", () => { + // A `lang === "mermaid"` / `"html"` comparison outside the dispatcher is a + // per-surface language branch — architecture constraint 2. + // + // The Tiptap NodeView is the one sanctioned exception (constraint 6): the + // EDITABLE code block owns its own preview-toggle lifecycle while the user + // types, and reuses the same leaf components rather than the readonly + // renderer. Rewriting the editor is explicitly out of scope for MUL-4922. + // Narrow, named, and justified — not a general loophole. + const TIPTAP_NODEVIEW = "editor/extensions/code-block-view.tsx"; + + const offenders = sourceFiles([...PRODUCT_SURFACES, "common", "editor"]) + .filter(({ path }) => path !== TIPTAP_NODEVIEW) + .filter(({ text }) => + /(?:lang|language)\w*\s*===\s*["'](?:mermaid|html)["']/.test(text), + ) + .map(({ path }) => path); + + expect(offenders).toEqual([]); + }); + + it("Tiptap reuses the leaf components but never imports RichContent", () => { + // Constraint 6: the editor keeps its own parser/lifecycle. Pulling the + // readonly renderer into the editor is the classic way this collapses. + const offenders = walk(join(VIEWS_ROOT, "editor")) + .filter((p) => !/readonly-content\.tsx$/.test(p)) + .map((path) => ({ path: relative(VIEWS_ROOT, path), text: readFileSync(path, "utf8") })) + .filter(({ text }) => /\bRichContent\b/.test(text)) + .map(({ path }) => path); + + expect(offenders).toEqual([]); + }); + + it("the canonical renderer stays in views, not ui", () => { + // Mention/Attachment/navigation are product concerns; moving this into + // packages/ui would break the ui -> no-core/views boundary. + const uiRoot = join(VIEWS_ROOT, "..", "ui"); + const offenders = walk(uiRoot) + .map((path) => ({ path, text: stripComments(readFileSync(path, "utf8")) })) + .filter(({ text }) => /\bRichContent\b|from\s+["']@multica\/views/.test(text)) + .map(({ path }) => relative(uiRoot, path)); + + expect(offenders).toEqual([]); + }); +}); + +describe("chat renders every text entry through RichContent", () => { + const chatList = readFileSync( + join(VIEWS_ROOT, "chat/components/chat-message-list.tsx"), + "utf8", + ); + + it("uses RichContent and no other markdown renderer", () => { + expect(chatList).toMatch(/\bRichContent\b/); + expect(chatList).not.toMatch(/MemoizedMarkdown| { + // The identity contract in source form: if someone reverts to `msg.id`, + // the parity test's remount assertion and this both fail. + expect(chatList).toMatch(/task:\$\{/); + expect(chatList).not.toMatch(/computeItemKey=\{\(_, msg\) => msg\.id\}/); + }); +}); diff --git a/packages/views/rich-content/rich-content-parity.test.tsx b/packages/views/rich-content/rich-content-parity.test.tsx new file mode 100644 index 00000000000..deef87cfe5f --- /dev/null +++ b/packages/views/rich-content/rich-content-parity.test.tsx @@ -0,0 +1,500 @@ +/** + * Five-surface RichContent parity (MUL-4922). + * + * The acceptance naiyuan set: ONE completed Markdown fixture must produce the + * SAME semantic blocks in Chat (user message, live assistant, persisted + * assistant), an Issue description and a Comment. Density/CSS may differ; + * capability may not. A Mermaid fence must be a diagram in all five, not a code + * block in some of them. + * + * The surfaces are exercised through their real entry points — ReadonlyContent + * for Issue/Comment and ChatMessageList for the three Chat rows — so a + * regression that reintroduces a Chat-only renderer fails here. + */ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { render, screen, waitFor, within } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ReactElement } from "react"; + +const { resolveIssueIdentifierMock, mermaidRenderMock } = vi.hoisted(() => ({ + resolveIssueIdentifierMock: vi.fn(), + mermaidRenderMock: vi.fn(), +})); + +vi.mock("../issues/hooks", () => ({ + useResolveIssueIdentifier: (identifier: string) => + resolveIssueIdentifierMock(identifier), +})); + +vi.mock("../i18n", async () => { + const editor = (await import("../locales/en/editor.json")).default; + const chat = (await import("../locales/en/chat.json")).default; + return { + useT: (ns?: string) => ({ + t: (select: (bundle: Record) => string) => + select((ns === "chat" ? chat : editor) as Record), + }), + useTimeAgo: () => "just now", + }; +}); + +vi.mock("@multica/core/api", () => ({ + api: { getAttachmentTextContent: vi.fn() }, + PreviewTooLargeError: class extends Error {}, + PreviewUnsupportedError: class extends Error {}, +})); + +vi.mock("@multica/core/paths", () => ({ + useWorkspacePaths: () => ({ + issueDetail: (id: string) => `/test/issues/${id}`, + projectDetail: (id: string) => `/test/projects/${id}`, + }), + useWorkspaceSlug: () => "test", +})); + +vi.mock("../navigation", () => ({ + useNavigation: () => ({ push: vi.fn(), openInNewTab: vi.fn() }), + AppLink: ({ href, children }: { href: string; children: React.ReactNode }) => ( + {children} + ), +})); + +vi.mock("../issues/components/issue-mention-card", () => ({ + IssueMentionCard: ({ issueId, fallbackLabel }: { issueId: string; fallbackLabel?: string }) => ( + {fallbackLabel ?? issueId} + ), +})); + +vi.mock("../projects/components/project-chip", () => ({ + ProjectChip: ({ projectId }: { projectId: string }) => ( + {projectId} + ), +})); + +vi.mock("../editor/link-hover-card", () => ({ + useLinkHover: () => ({}), + LinkHoverCard: () => null, +})); + +vi.mock("mermaid", () => ({ + default: { + initialize: vi.fn(), + render: mermaidRenderMock, + }, +})); + +// react-virtuoso does not virtualize under jsdom's zero-height viewport, so +// render every row directly. computeItemKey is still exercised: it is what +// gives the live row and the persisted row one identity. +vi.mock("react-virtuoso", () => ({ + Virtuoso: ({ + data, + itemContent, + computeItemKey, + components, + context, + }: { + data: unknown[]; + itemContent: (i: number, item: unknown) => ReactElement; + computeItemKey: (i: number, item: unknown) => string; + components?: { Footer?: (p: { context?: unknown }) => ReactElement | null }; + context?: unknown; + }) => { + const Footer = components?.Footer; + return ( +
+ {data.map((item, i) => ( +
+ {itemContent(i, item)} +
+ ))} + {Footer ?
: null} +
+ ); + }, +})); + +Object.defineProperty(HTMLCanvasElement.prototype, "getContext", { + value: () => ({ + fillStyle: "#000", + fillRect: vi.fn(), + getImageData: () => ({ data: new Uint8ClampedArray([12, 34, 56, 255]) }), + }), +}); + +import { ReadonlyContent } from "../editor/readonly-content"; +import { ChatMessageList } from "../chat/components/chat-message-list"; +import { taskMessagesOptions } from "@multica/core/chat/queries"; + +// naiyuan's fixture, corrected to a single closed fence. +const MERMAID_FIXTURE = [ + "```mermaid", + "flowchart LR", + ' HTML["HTML"] --> WEB["网页"]', + ' CSS["CSS"] --> WEB', + "```", +].join("\n"); + +const TASK_ID = "11111111-1111-4111-8111-111111111111"; + +function makeClient(): QueryClient { + return new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: 0 } }, + }); +} + +function withClient(ui: ReactElement, client: QueryClient) { + return {ui}; +} + +/** A persisted assistant chat_message carrying `content`. */ +function assistantMessage(content: string) { + return { + id: "msg-assistant-1", + role: "assistant" as const, + content, + task_id: TASK_ID, + attachments: [], + elapsed_ms: 1200, + }; +} + +function userMessage(content: string) { + return { + id: "msg-user-1", + role: "user" as const, + content, + task_id: null, + attachments: [], + }; +} + +/** Seed the task-messages cache the way useRealtimeSync does during a run. */ +function seedTimeline(client: QueryClient, text: string) { + client.setQueryData(taskMessagesOptions(TASK_ID).queryKey, [ + { + task_id: TASK_ID, + issue_id: null, + seq: 1, + type: "text", + content: text, + }, + ] as never); +} + +beforeEach(() => { + vi.clearAllMocks(); + mermaidRenderMock.mockResolvedValue({ + svg: 'diagram', + }); + resolveIssueIdentifierMock.mockReturnValue(null); +}); + +/** + * The Mermaid leaf's own container — NOT a bare `svg` query, which also matches + * the lucide icons in code-block chrome. + */ +const mermaidLeaf = (container: HTMLElement): Element | null => + container.querySelector(".mermaid-diagram"); + +/** Every surface must expose the diagram through the same Mermaid leaf. */ +async function expectMermaidRendered(container: HTMLElement) { + await waitFor(() => { + expect(mermaidLeaf(container)).not.toBeNull(); + }); + await waitFor(() => { + expect(mermaidLeaf(container)?.querySelector("svg")).not.toBeNull(); + }); + // Not a plain code block: the fence was upgraded. + expect(container.querySelector("code.hljs")).toBeNull(); +} + +describe("Mermaid parity across the five surfaces", () => { + it("renders a diagram in an Issue description", async () => { + const { container } = render(); + await expectMermaidRendered(container); + }); + + it("renders a diagram in a Comment", async () => { + // Comment and Issue description share ReadonlyContent; asserting both keeps + // the acceptance list honest about what was actually exercised. + const { container } = render( + , + ); + await expectMermaidRendered(container); + }); + + it("renders a diagram in a Chat user message", async () => { + const client = makeClient(); + const { container } = render( + withClient( + , + client, + ), + ); + await expectMermaidRendered(container); + }); + + it("renders a diagram in a persisted Chat assistant message", async () => { + const client = makeClient(); + seedTimeline(client, MERMAID_FIXTURE); + const { container } = render( + withClient( + , + client, + ), + ); + await expectMermaidRendered(container); + }); + + it("renders a diagram in a live (streaming) Chat assistant row", async () => { + const client = makeClient(); + seedTimeline(client, MERMAID_FIXTURE); + const { container } = render( + withClient( + , + client, + ), + ); + await expectMermaidRendered(container); + }); +}); + +describe("streaming fence gate in Chat", () => { + it("does not instantiate Mermaid while the fence is still open", async () => { + const client = makeClient(); + // Same content, closing fence not yet streamed. + seedTimeline(client, "```mermaid\nflowchart LR\n A --> B\n"); + const { container } = render( + withClient( + , + client, + ), + ); + + // Source is shown as ordinary code; Mermaid is never called. + await waitFor(() => { + expect(container.querySelector("code")).not.toBeNull(); + }); + expect(mermaidRenderMock).not.toHaveBeenCalled(); + expect(mermaidLeaf(container)).toBeNull(); + }); + + it("upgrades in place once the closing fence arrives", async () => { + const client = makeClient(); + seedTimeline(client, "```mermaid\nflowchart LR\n A --> B\n"); + const { container, rerender } = render( + withClient( + , + client, + ), + ); + expect(mermaidRenderMock).not.toHaveBeenCalled(); + + // Closing fence streams in. + seedTimeline(client, "```mermaid\nflowchart LR\n A --> B\n```"); + rerender( + withClient( + , + client, + ), + ); + + await waitFor(() => { + expect(mermaidLeaf(container)).not.toBeNull(); + }); + }); + + it("keeps a settled-but-unclosed fence as source", async () => { + // Task finished, fence still malformed: completion must not bypass the gate. + const client = makeClient(); + const { container } = render( + withClient( + B\n")] as never} + pendingTask={null} + availability={undefined} + />, + client, + ), + ); + await waitFor(() => { + expect(container.querySelector("code")).not.toBeNull(); + }); + expect(mermaidRenderMock).not.toHaveBeenCalled(); + }); +}); + +describe("live → persisted row identity", () => { + // Howard's contract #1: the handoff must be an in-place data swap, not a + // remount, or every completed task rebuilds its diagrams and the scroll + // position jumps. + it("keeps one row key across the handoff and does not re-run Mermaid", async () => { + const client = makeClient(); + seedTimeline(client, MERMAID_FIXTURE); + + const live = ( + + ); + const { container, rerender } = render(withClient(live, client)); + + await waitFor(() => expect(mermaidLeaf(container)).not.toBeNull()); + const liveKey = container.querySelector("[data-row-key]")?.getAttribute("data-row-key"); + const rendersWhileLive = mermaidRenderMock.mock.calls.length; + expect(liveKey).toBe(`task:${TASK_ID}`); + + // The assistant message persists; the pending task clears. + rerender( + withClient( + , + client, + ), + ); + + await waitFor(() => { + expect(screen.getByText(/Replied in/i)).toBeInTheDocument(); + }); + + // Same key => React reconciled the row instead of remounting it. + const persistedKey = container + .querySelector("[data-row-key]") + ?.getAttribute("data-row-key"); + expect(persistedKey).toBe(liveKey); + + // And the diagram was not re-rendered by the handoff. + expect(mermaidRenderMock.mock.calls.length).toBe(rendersWhileLive); + expect(mermaidLeaf(container)).not.toBeNull(); + }); + + it("gives a persisted assistant message a task-scoped row key", () => { + const client = makeClient(); + const { container } = render( + withClient( + , + client, + ), + ); + expect(container.querySelector("[data-row-key]")?.getAttribute("data-row-key")).toBe( + `task:${TASK_ID}`, + ); + }); +}); + +describe("semantic parity beyond Mermaid", () => { + const FIXTURE = [ + "A [link](https://example.com) and a mention [MUL-7](mention://issue/MUL-7).", + "", + "```html", + "preview", + "```", + "", + "```ts", + "const a = 1;", + "```", + "", + "highlighted", + ].join("\n"); + + function renderReadonly() { + return render().container; + } + + function renderChatUser() { + const client = makeClient(); + return render( + withClient( + , + client, + ), + ).container; + } + + it("produces the same block set in Issue/Comment and Chat", async () => { + resolveIssueIdentifierMock.mockImplementation((id: string) => + id === "MUL-7" ? { id: "issue-7", identifier: "MUL-7" } : null, + ); + + const readonly = renderReadonly(); + const chat = renderChatUser(); + + for (const container of [readonly, chat]) { + // HTML fence -> sandboxed preview iframe (not a code block) + await waitFor(() => { + expect(container.querySelector("iframe")).not.toBeNull(); + }); + expect(container.querySelector("iframe")?.getAttribute("sandbox")).toBe( + "allow-scripts", + ); + // Ordinary language -> lowlight static code + expect(container.querySelector("code.hljs")).not.toBeNull(); + // Mention chip + expect(within(container).getByTestId("issue-mention")).toBeInTheDocument(); + // Highlight + expect(container.querySelector("mark")?.textContent).toBe("highlighted"); + // Plain link + expect(container.querySelector('a[href="https://example.com"]')).not.toBeNull(); + } + }); + + it("does not dispatch htmlbars / mermaidx to rich blocks on either surface", async () => { + const near = "```htmlbars\nx\n```\n\n```mermaidx\ngraph TD\n```"; + const readonly = render().container; + const client = makeClient(); + const chat = render( + withClient( + , + client, + ), + ).container; + + for (const container of [readonly, chat]) { + expect(container.querySelector("iframe")).toBeNull(); + expect(container.querySelectorAll("code.hljs").length).toBe(2); + } + expect(mermaidRenderMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/views/rich-content/rich-content.css b/packages/views/rich-content/rich-content.css new file mode 100644 index 00000000000..bd7f9ac3ad5 --- /dev/null +++ b/packages/views/rich-content/rich-content.css @@ -0,0 +1,37 @@ +/* + * RichContent density modifier. + * + * `density` is CSS-only (MUL-4922): compact and document render the SAME + * semantic DOM through the same components map, and differ only in spacing. + * Nothing here may change which blocks exist — only how tightly they sit. + * + * Compact is used by Chat, where a message bubble should not carry the + * leading/trailing block margins a document body wants, and consecutive + * paragraphs sit closer together. + */ + +.rich-text-editor.rich-content-compact > :first-child { + margin-top: 0; +} + +.rich-text-editor.rich-content-compact > :last-child { + margin-bottom: 0; +} + +.rich-content-compact p { + margin-top: 0.375rem; + margin-bottom: 0.375rem; +} + +.rich-content-compact ul, +.rich-content-compact ol { + margin-top: 0.375rem; + margin-bottom: 0.375rem; +} + +/* Code blocks and rich blocks keep a little more air than prose so a diagram + or preview does not collide with the surrounding text. */ +.rich-content-compact .code-block-wrapper { + margin-top: 0.5rem; + margin-bottom: 0.5rem; +} diff --git a/packages/views/rich-content/rich-content.tsx b/packages/views/rich-content/rich-content.tsx new file mode 100644 index 00000000000..6991f3d4740 --- /dev/null +++ b/packages/views/rich-content/rich-content.tsx @@ -0,0 +1,429 @@ +"use client"; + +/** + * RichContent — the ONE product-level readonly content renderer (MUL-4922). + * + * Chat (user message, live assistant, persisted assistant, timeline text), + * Issue descriptions and Comments all render through this component. There is + * no second product Markdown renderer: `packages/ui/markdown` stays a generic + * primitive for terminal/raw output and must not grow product concerns + * (Mention, Attachment, Mermaid, HTML preview) again. + * + * Surfaces differ only in the shell they wrap around this component: + * - Comment / Issue: edit, reply, menu, thread chrome — outside. + * - Chat: timeline, thinking, tool, failure, copy chrome — outside. + * + * The public API is deliberately narrow. There is no `surface` prop, no + * `renderMention` override and no custom code-renderer escape hatch: each of + * those is a door through which a per-surface fork walks back in. Link, + * mention and attachment behaviour is decided here, once. + * + * density — CSS only. Never switches parser, plugins, components map or the + * semantic DOM; `compact` and `document` produce the same blocks. + * phase — lifecycle only. Does NOT decide whether a fence upgrades; that is + * the fence's real closed state (see streaming-fence.ts), so a + * settled-but-malformed fence still renders as source. + */ + +import { createContext, isValidElement, memo, useContext, useMemo, useRef } from "react"; +import ReactMarkdown, { type Components } from "react-markdown"; +import type { ReactNode } from "react"; +import rehypeKatex from "rehype-katex"; +import remarkBreaks from "remark-breaks"; +import remarkGfm from "remark-gfm"; +import remarkMath from "remark-math"; +import rehypeRaw from "rehype-raw"; +import rehypeSanitize from "rehype-sanitize"; +import { cn } from "@multica/ui/lib/utils"; +import { useWorkspacePaths, useWorkspaceSlug } from "@multica/core/paths"; +import type { Attachment } from "@multica/core/types"; +import { + isAllowedFileCardHref, + isIssueIdentifier, + markdownSanitizeSchema, + markdownUrlTransform, +} from "@multica/ui/markdown"; +import { useNavigation } from "../navigation"; +import { IssueMentionCard } from "../issues/components/issue-mention-card"; +import { useResolveIssueIdentifier } from "../issues/hooks"; +import { ProjectChip } from "../projects/components/project-chip"; +import { useLinkHover, LinkHoverCard } from "../editor/link-hover-card"; +import { openLink, isMentionHref } from "../editor/utils/link-handler"; +import { preprocessMarkdown } from "../editor/utils/preprocess"; +import { highlightToHtml } from "../editor/utils/highlight-markdown"; +import { AttachmentDownloadProvider } from "../editor/attachment-download-context"; +import { Attachment as AttachmentRenderer } from "../editor/attachment"; +import { computeClosedFenceOffsets } from "./streaming-fence"; +import { + CodeBlockShell, + RichFenceBlock, + StaticCodeBody, + isRichFenceLanguage, + shouldUpgradeFence, +} from "./rich-code-block"; +import "katex/dist/katex.min.css"; +import "../editor/styles/index.css"; +import "./rich-content.css"; + +export type RichContentDensity = "compact" | "document"; +export type RichContentPhase = "streaming" | "settled"; + +// --------------------------------------------------------------------------- +// Fence gate context +// --------------------------------------------------------------------------- +// +// The closed-fence offsets travel by context rather than by rebuilding the +// components map. The map must stay referentially stable: react-markdown +// re-runs the whole subtree when `components` changes identity, which rewrites +// every highlighted 's innerHTML and collapses an active text selection +// inside a code block (MUL-3621). + +const ClosedFenceContext = createContext>(new Set()); + +function useIsFenceClosed(offset: number | undefined): boolean { + const closed = useContext(ClosedFenceContext); + // An offset-less node cannot be matched to a source fence; refusing to + // upgrade is the safe direction (source instead of a possibly partial block). + return offset != null && closed.has(offset); +} + +// --------------------------------------------------------------------------- +// Mention / link renderers +// --------------------------------------------------------------------------- + +/** + * Issue mention chip. Navigation — plain click, modifier click, and the + * "open issue links in new tab" preference — is owned by the AppLink inside + * IssueMentionCard; the wrapper only shields surrounding click handlers + * (e.g. collapsed-comment expanders) from mention clicks. + */ +function IssueMentionLink({ issueId, label }: { issueId: string; label?: string }) { + return ( + e.stopPropagation()}> + + + ); +} + +/** + * Autolinked bare identifier (e.g. `MUL-123`) routed through + * `mention://issue/` by the preprocessor. Resolves to a real issue + * in the current workspace; renders a navigable mention on a hit, plain text on + * a miss / while loading / cross-workspace. + */ +function AutolinkedIssueMentionLink({ identifier }: { identifier: string }) { + const issue = useResolveIssueIdentifier(identifier); + if (!issue) return <>{identifier}; + return ; +} + +function ProjectMentionLink({ projectId, label }: { projectId: string; label?: string }) { + const { push, openInNewTab } = useNavigation(); + const p = useWorkspacePaths(); + const path = p.projectDetail(projectId); + return ( + { + e.preventDefault(); + e.stopPropagation(); + if (e.metaKey || e.ctrlKey || e.shiftKey) { + if (openInNewTab) { + openInNewTab(path, label); + } + return; + } + push(path); + }} + > + + + ); +} + +function childrenToLabel(children: ReactNode): string | undefined { + if (typeof children === "string") return children; + if (Array.isArray(children)) return children.join(""); + return undefined; +} + +function RichLink({ href, children }: { href?: string; children?: ReactNode }) { + const slug = useWorkspaceSlug(); + + if (href?.startsWith("slash://skill/")) { + return {children}; + } + + if (isMentionHref(href)) { + const match = href.match(/^mention:\/\/(member|agent|issue|project|all)\/(.+)$/); + if (match?.[1] === "issue" && match[2]) { + // A bare identifier (from the autolink preprocessor) is carried as the id + // segment; a real mention carries a UUID. Dispatch on the id shape. + if (isIssueIdentifier(match[2])) { + return ; + } + return ; + } + if (match?.[1] === "project" && match[2]) { + return ; + } + // Member / agent / all mentions + return {children}; + } + + // Regular links — open directly on click + return ( + { + e.preventDefault(); + if (href) openLink(href, slug); + }} + > + {children} + + ); +} + +// --------------------------------------------------------------------------- +// Fenced code +// --------------------------------------------------------------------------- + +function getTextContent(node: ReactNode): string { + if (node == null || typeof node === "boolean") return ""; + if (typeof node === "string" || typeof node === "number") return String(node); + if (Array.isArray(node)) return node.map(getTextContent).join(""); + if (isValidElement(node)) { + const props = node.props as { children?: ReactNode }; + return getTextContent(props.children); + } + return ""; +} + +/** + * `code` renderer. Returns the rich leaf directly for an upgraded fence and + * relies on the `pre` renderer below to unwrap it — react-markdown otherwise + * wraps `code` children in a `
` whose monospace/overflow styles clamp a
+ * diagram or preview iframe.
+ */
+function RichCode({
+  className,
+  children,
+  node,
+  ...props
+}: {
+  className?: string;
+  children?: ReactNode;
+  node?: { position?: { start: { offset?: number; line: number }; end: { line: number } } };
+} & Record) {
+  const language = /language-(\w+)/.exec(className || "")?.[1];
+  const isBlock =
+    node?.position && node.position.start.line !== node.position.end.line;
+  const isFenceClosed = useIsFenceClosed(node?.position?.start.offset);
+
+  if (isBlock && shouldUpgradeFence(language, isFenceClosed)) {
+    // isRichFenceLanguage is re-checked for the type narrow; shouldUpgradeFence
+    // already required it.
+    if (isRichFenceLanguage(language)) {
+      return ;
+    }
+  }
+
+  if (!isBlock && !language) {
+    // Inline code — CSS handles styling via .rich-text-editor code
+    return {children};
+  }
+
+  return ;
+}
+
+/**
+ * `pre` renderer. react-markdown calls this BEFORE invoking the `code`
+ * renderer, so `children` is the unrendered `` element from the AST: the
+ * decision to unwrap is made from that child's own hast node (class token +
+ * source offset), not by checking `children.type === MermaidDiagram`, which
+ * never matches.
+ */
+function RichPre({ children }: { children?: ReactNode }) {
+  const child = Array.isArray(children) ? children[0] : children;
+  let language: string | undefined;
+  let offset: number | undefined;
+
+  if (isValidElement(child)) {
+    const childProps = child.props as {
+      className?: string;
+      node?: { position?: { start: { offset?: number } } };
+    };
+    language = /(?:^|\s)language-(\w+)(?:\s|$)/.exec(childProps.className ?? "")?.[1];
+    offset = childProps.node?.position?.start.offset;
+  }
+
+  const isFenceClosed = useIsFenceClosed(offset);
+
+  // Upgraded fences escape the 
 envelope entirely. An OPEN
+  // mermaid/html fence deliberately falls through to the normal code shell, so
+  // a half-streamed diagram reads as ordinary source with copy chrome.
+  if (shouldUpgradeFence(language, isFenceClosed)) {
+    return <>{children};
+  }
+
+  return (
+    
+      {children}
+    
+  );
+}
+
+// The components map is module-level and static: it never depends on density,
+// phase or the fence gate, so its identity is stable for the lifetime of the
+// app and react-markdown can bail out of unchanged subtrees.
+const COMPONENTS: Partial = {
+  a: RichLink,
+
+  // Images — unified through . The resolver context provided by
+  // AttachmentDownloadProvider turns a CDN URL into a full record when
+  // possible; external URLs render as plain images with lightbox-via-preview
+  // -modal. forceKind is mandatory: markdown `![]()` carries no content-type
+  // and alt is commonly empty, so without it images fall through to file-card
+  // chrome.
+  img: ({ src, alt }) => (
+    
+  ),
+
+  // FileCard — intercept 
from preprocessMarkdown + div: ({ node, children, ...props }) => { + const dataType = node?.properties?.dataType as string | undefined; + if (dataType === "fileCard") { + const rawHref = (node?.properties?.dataHref as string) || ""; + const href = isAllowedFileCardHref(rawHref) ? rawHref : ""; + const filename = (node?.properties?.dataFilename as string) || ""; + return ; + } + return
{children}
; + }, + + // Tables — wrap in tableWrapper div for border/radius/scroll (matches Tiptap) + table: ({ children }) => ( +
+ {children}
+
+ ), + + code: RichCode as NonNullable, + pre: RichPre as NonNullable, +}; + +const REMARK_PLUGINS = [ + [remarkMath, { singleDollarTextMath: false }], + remarkBreaks, + [remarkGfm, { singleTilde: false }], +] as never; + +const REHYPE_PLUGINS = [ + rehypeRaw, + [rehypeSanitize, markdownSanitizeSchema], + rehypeKatex, +] as never; + +// --------------------------------------------------------------------------- +// Component +// --------------------------------------------------------------------------- + +export interface RichContentProps { + content: string; + /** + * Attachments associated with the surrounding entity (comment / issue body / + * chat message). Inline `` and file-card URLs matching one of these get + * their download URL re-signed at click time instead of using the possibly + * stale link embedded in the markdown. + * + * Callers SHOULD pass a stable reference; a fresh array on every parent + * render busts the memo. + */ + attachments?: Attachment[]; + /** CSS density only — never changes the semantic DOM. */ + density?: RichContentDensity; + /** + * Streaming lifecycle. Does not gate fence upgrades (closedness does); it + * exists so surfaces can express that content is still arriving. + */ + phase?: RichContentPhase; + className?: string; +} + +export const RichContent = memo(function RichContent({ + content, + attachments, + density = "document", + phase = "settled", + className, +}: RichContentProps) { + const processed = useMemo( + () => highlightToHtml(preprocessMarkdown(content, { autolinkIssueIdentifiers: true })), + [content], + ); + + // Derived from the SAME string handed to ReactMarkdown, so offsets line up + // with the hast node positions the `code`/`pre` renderers observe. Computing + // it from the raw pre-preprocess text would mis-match every rewritten node. + const closedFences = useMemo(() => computeClosedFenceOffsets(processed), [processed]); + + const wrapperRef = useRef(null); + const hover = useLinkHover(wrapperRef); + + // Memoize the react-markdown subtree on its only real inputs. Unrelated + // parent re-renders (a sibling agent task streaming over WebSocket fires one + // every ~100ms) would otherwise re-run react-markdown, which hands `` a + // fresh `dangerouslySetInnerHTML` object each time; React then rewrites the + // highlighted innerHTML even though the string is byte-identical, tearing + // down every hljs and collapsing any active text selection inside a + // code block (MUL-3621). A stable element reference lets React bail out. + const markdown = useMemo( + () => ( + + + {processed} + + + ), + [processed, closedFences], + ); + + return ( + +
+ {markdown} + +
+
+ ); +}); diff --git a/packages/views/rich-content/streaming-fence.test.ts b/packages/views/rich-content/streaming-fence.test.ts new file mode 100644 index 00000000000..fc0747af5db --- /dev/null +++ b/packages/views/rich-content/streaming-fence.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from "vitest"; +import { computeClosedFenceOffsets } from "./streaming-fence"; + +const isClosed = (src: string, offset = 0): boolean => + computeClosedFenceOffsets(src).has(offset); + +describe("computeClosedFenceOffsets", () => { + it("treats a closed backtick fence as closed", () => { + expect(isClosed("```mermaid\ngraph TD\n```\n")).toBe(true); + }); + + it("treats an unterminated fence as open", () => { + expect(isClosed("```mermaid\ngraph TD\n")).toBe(false); + }); + + it("treats a bare opener with no content as open", () => { + expect(isClosed("```mermaid\n")).toBe(false); + }); + + // The streaming case that matters: text keeps arriving after the opener but + // the closer has not landed yet. + it("stays open while content accumulates", () => { + expect(isClosed("```mermaid\nflowchart LR\n A --> B\n B --> C\n")).toBe(false); + }); + + it("handles tilde fences", () => { + expect(isClosed("~~~mermaid\ngraph TD\n~~~\n")).toBe(true); + expect(isClosed("~~~mermaid\ngraph TD\n")).toBe(false); + }); + + // CommonMark: a closer must use the same character as the opener. + it("does not accept a tilde closer for a backtick opener", () => { + expect(isClosed("```mermaid\ngraph TD\n~~~\n")).toBe(false); + }); + + // CommonMark: a closer must be at least as long as the opener. This is the + // case a naive "line of >=3 markers" check reports closed, which would hand + // Mermaid a half-written diagram. + it("does not accept a shorter closer than the opener", () => { + expect(isClosed("````mermaid\ngraph TD\n```\n")).toBe(false); + }); + + it("accepts a longer closer than the opener", () => { + expect(isClosed("```mermaid\ngraph TD\n`````\n")).toBe(true); + }); + + it("handles an indented fence", () => { + expect(isClosed(" ```mermaid\n graph TD\n ```\n", 2)).toBe(true); + }); + + it("handles a fence inside a list item", () => { + const src = "- item\n\n ```mermaid\n graph TD\n ```\n"; + expect(computeClosedFenceOffsets(src).size).toBe(1); + expect(isClosed(src, src.indexOf("```"))).toBe(true); + }); + + // A ```` md block whose *content* contains a ``` mermaid fence is one code + // node, not two — the inner fence must not register as its own block. + it("does not report an inner fence of a nested block", () => { + const src = "````md\n```mermaid\ngraph TD\n```\n````\n"; + const offsets = computeClosedFenceOffsets(src); + expect(offsets.size).toBe(1); + expect(offsets.has(0)).toBe(true); + }); + + it("reports each block independently when several are present", () => { + const src = "```mermaid\ngraph TD\n```\n\ntext\n\n```html\nx\n"; + const offsets = computeClosedFenceOffsets(src); + expect(offsets.has(0)).toBe(true); + expect(offsets.has(src.lastIndexOf("```html"))).toBe(false); + }); + + // The realistic streaming progression: one source, growing one chunk at a + // time. The block must flip to closed exactly once, at the closer. + it("flips to closed exactly when the closing fence arrives", () => { + const steps = [ + "```mermaid", + "```mermaid\n", + "```mermaid\nflowchart LR\n", + "```mermaid\nflowchart LR\n A --> B\n", + "```mermaid\nflowchart LR\n A --> B\n``", + "```mermaid\nflowchart LR\n A --> B\n```", + ]; + const results = steps.map((s) => isClosed(s)); + expect(results).toEqual([false, false, false, false, false, true]); + }); + + it("ignores an indented (non-fenced) code block", () => { + // Four-space indented code has no fence to dangle; it carries no info + // string so it can never dispatch to a rich block. + const src = " const a = 1\n"; + expect(computeClosedFenceOffsets(src).size).toBe(1); + }); + + it("returns an empty set for empty input", () => { + expect(computeClosedFenceOffsets("").size).toBe(0); + }); + + it("returns an empty set for prose with no code", () => { + expect(computeClosedFenceOffsets("just some **text**\n").size).toBe(0); + }); +}); diff --git a/packages/views/rich-content/streaming-fence.ts b/packages/views/rich-content/streaming-fence.ts new file mode 100644 index 00000000000..7c9a71c2f9f --- /dev/null +++ b/packages/views/rich-content/streaming-fence.ts @@ -0,0 +1,105 @@ +import { fromMarkdown } from "mdast-util-from-markdown"; +import type { Root, RootContent, Code } from "mdast"; + +/** + * Fenced-code closedness gate for streaming RichContent (MUL-4922). + * + * A Mermaid diagram or sandboxed HTML iframe must never be instantiated from a + * fence the author is still typing: mid-stream the source is syntactically + * incomplete, so Mermaid would throw on every keystroke and an iframe would be + * created and destroyed dozens of times per second. Only a *closed* fence may + * upgrade to a rich block. + * + * This module answers exactly one question — "which fenced code blocks in this + * source are closed?" — and returns offsets. It deliberately does NOT render. + * Rendering stays in the single ReactMarkdown pipeline; a gate that rendered + * would become the second renderer this sweep exists to delete. + * + * Closedness is derived from a real CommonMark parse rather than a + * `startsWith("```")` scan, so indented fences, tilde fences, longer-than-three + * markers, fences inside list items, and nested fences are all judged the way + * the actual Markdown parser judges them. + */ + +// Opening fence: up to 3 leading spaces, then >= 3 backticks or tildes. +const FENCE_OPEN_RE = /^ {0,3}(`{3,}|~{3,})/; + +function isCodeNode(node: RootContent | Root): node is Code { + return node.type === "code"; +} + +function collectCodeNodes(node: Root | RootContent, out: Code[]): void { + if (isCodeNode(node)) { + out.push(node); + return; + } + const children = (node as { children?: RootContent[] }).children; + if (children) { + for (const child of children) collectCodeNodes(child, out); + } +} + +/** + * True when `raw` (the exact source span of one mdast `code` node) ends with a + * closing fence valid for its own opener. + * + * CommonMark requires the closer to use the same character as the opener and be + * at least as long, so an opener of ```` closed by ``` is still OPEN — the + * shorter run is content, not a terminator. Length is checked explicitly + * because a regex that only matched "a line of >=3 markers" reports that case + * closed and would let a half-written diagram reach Mermaid. + */ +function endsWithClosingFence(raw: string): boolean { + const lines = raw.split("\n"); + const openMatch = FENCE_OPEN_RE.exec(lines[0] ?? ""); + // No opener means an indented (4-space) code block. It has no fence to leave + // dangling, and carries no info string, so it can never dispatch to a rich + // block — treat it as settled. + if (!openMatch?.[1]) return true; + + const openFence = openMatch[1]; + const marker = openFence[0] as "`" | "~"; + // A single line cannot be both opener and closer. + if (lines.length < 2) return false; + + // mdast may or may not include the trailing newline in the node span. + const lastLine = (lines[lines.length - 1] === "" ? lines[lines.length - 2] : lines[lines.length - 1]) ?? ""; + const closeMatch = /^ {0,3}([`~]+)[ \t]*$/.exec(lastLine); + if (!closeMatch?.[1]) return false; + + const closeFence = closeMatch[1]; + return closeFence[0] === marker && closeFence.length >= openFence.length; +} + +/** + * Start offsets of every fenced code block that is closed in `source`. + * + * `source` MUST be the final processed Markdown handed to ReactMarkdown — the + * same string preprocess/highlight already rewrote. Offsets computed against + * the raw pre-preprocess text would drift and mis-match the wrong node. + */ +export function computeClosedFenceOffsets(source: string): Set { + const closed = new Set(); + if (!source) return closed; + + let tree: Root; + try { + tree = fromMarkdown(source); + } catch { + // A parse failure must not upgrade anything: fall back to "nothing is + // closed", which shows source instead of running Mermaid on bad input. + return closed; + } + + const codes: Code[] = []; + collectCodeNodes(tree, codes); + + for (const node of codes) { + const start = node.position?.start.offset; + const end = node.position?.end.offset; + if (start == null || end == null) continue; + if (endsWithClosingFence(source.slice(start, end))) closed.add(start); + } + + return closed; +} diff --git a/packages/views/skills/components/file-viewer.tsx b/packages/views/skills/components/file-viewer.tsx index 9457bfc5dfc..38fa639fcdc 100644 --- a/packages/views/skills/components/file-viewer.tsx +++ b/packages/views/skills/components/file-viewer.tsx @@ -9,7 +9,7 @@ import { parseFrontmatter, type SkillFrontmatter, } from "@multica/core/skills/frontmatter"; -import { Markdown } from "../../common/markdown"; +import { RichContent } from "../../rich-content"; import { useT } from "../../i18n"; function isMarkdown(path: string) { @@ -102,9 +102,11 @@ export function FileViewer({ {isMd && !editing ? (
{frontmatter && } - - {body || t(($) => $.file_viewer.no_content)} - + $.file_viewer.no_content)} + density="document" + phase="settled" + />
) : (