Skip to content

feat(webui): transcript 渲染 Mermaid 围栏(#125 重提:新路径 + 主题 + 净化) - #224

Merged
modacker merged 2 commits into
webuifrom
fix/webui-mermaid-fence-v2
Oct 22, 2026
Merged

modacker merged 2 commits into
webuifrom
fix/webui-mermaid-fence-v2

Conversation

@stevenjj33

Copy link
Copy Markdown
Collaborator

这是 #125 的重提

按 modacker 在 #125 上的评审意见重做。#125 关掉,那条 PR 的内容分三处:路径全要重定位、三处硬门槛、自身的缺陷。逐条处理如下。

路径重定位

旧路径 现在的路径
components/transcript/markdown.tsx features/transcript/ui/markdown.public.tsx
components/transcript/webui-mermaid.tsx features/transcript/ui/webui-mermaid.tsx
styles/shell.css(.webui-mermaid 区块) features/transcript/ui/transcript.css
test/webui-browser/server.mjs packages/webui/test/browser/server.mjs
test/webui-browser/transcript.spec.mjs packages/webui/test/browser/transcript.spec.mjs
scripts/webui-dependency-rules.mjs scripts/lib/webui-dependency-rules.mjs

三处硬门槛

  1. base 的浏览器测试服务器不 Serve assets/ → 已加(packages/webui/test/browser/server.mjs 现在按 /assets/ 前缀取文件,附带 svg 的 content-type)。真实服务早就 Serve(server/http/client-assets.ts),只有测试服务器缺。
  2. splitting: true 是全局副作用 → 两个构建脚本都开了(scripts/build-webui.mjs 与 scripts/package-webui-npm.mjs)。composer 的斜杠面板本来就是动态 import,现在也被切进 chunk——它的浏览器用例(composer.spec.mjs「typing / opens the slash palette」)就是 chunk 化的回归覆盖,全套浏览器 151 条里它绿着。
  3. 发布门会审 markdown.public.tsx → import("mermaid") 是裸 specifier,不算 foreign;本文件还登记进了冻结的路径元数据 oracle(packages/webui/test/gates/fixtures/path-metadata-f00.json),依赖门 117/117 绿。

自身的缺陷,三处都修了

主题完全没接。 Mermaid 把调色板烤进返回的 SVG,所以用浅色调色板画的图在深色页面上仍是一张浅色的图——评审说的「割裂」。现在渲染时用应用自己的主题(经注入的 DOM adapter readDocumentTheme 读 <html> 上的 dark class;依赖规则把 ambient document 限定在 platform 层,组件不许直接碰,所以走 adapter 而不是自己读 document)。主题变化时库会重新初始化,下一张图跟着变。

不可信内容直接进渲染器。 图的正文是模型输出,渲染出的 SVG 又用 dangerouslySetInnerHTML 注入,所以 SVG 进 DOM 前先过滤:script 元素、on* 事件属性、javascript: 之类的 URL、foreignObject 这类外部文档全部去掉,图形本身不动。Mermaid 自己的 securityLevel: "strict" 仍是第一道防线,过滤是第二道。同时关掉 htmlLabels——图变成纯 SVG,标签是被转义的 <text>,而不是塞在 foreignObject 里的 HTML。

测试只覆盖 happy path。 单测全 mock mermaid(Node 没 DOM),.catch() 失败分支没有覆盖,真库只在浏览器里跑过一次成功路径。现在浏览器用例跑真库,覆盖:真渲染出 SVG、无效语法回退到普通围栏、图跟随应用主题、宽图不撑破容器、流式半截围栏回退而不弄坏 transcript。净化与主题读取有单测。

两处夹带,这次没有

旧 PR 里的 composer store 适配器提交(上游已自行实现)、空的 ci: retrigger 提交、以及 lockfile 里的 yaml 2.9.0 → 2.9.1 升级,这次都没有:lockfile diff 是纯新增 853 行、零删除(mermaid 的依赖树),单提交无夹带。

产物大小(对当前 webui HEAD 实测)

webui HEAD 本 PR
入口 client.js 4,647,161 B 4,456,586 B(小 190 KB)
懒加载 chunk 无 109 个(assets/,共 12 MB)

入口不增反降(splitting 把共享代码拆进 chunk),mermaid 全部延迟加载。

Validation(本地全过)

  • pnpm build:webui
  • pnpm --filter @mavis/webui typecheck:client + typecheck:test:0 错误
  • 新单测 22/22;依赖门 117/117;全量单测 442 passed / 2 failed(webui-w0-css-structure 两条,对照 origin/webui 同样红,HEAD 自带)
  • npx playwright test packages/webui/test/browser:151 passed / 7 skipped(含 5 条新 mermaid 用例与斜杠面板的 chunk 回归)

CI

仓库 Actions 已恢复。今天 webui 分支自己的 Source verification 也是 failure(与那 2 条同源),本 PR 的 CI 结果请按此背景读。

@stevenjj33

Copy link
Copy Markdown
Collaborator Author

关于 CI 失败的两层分析(重要)

本 PR 的 ubuntu(及 rerun)失败分两层,需要分开看:

第一层:webui-w0-css-structure 两条 —— 全仓既有红,与本 PR 无关

今天 07:16 合入 webui 的 fix/browser-panel-performance 自己的 CI 就挂着这两条(run 39739305355,ubuntu+macos 都红),但仍然被合入了。我在本地把工作树还原到 origin/webui 对照跑:同样 2 failed / 27 passed。这两条是冻结 CSS 结构门的期望漂移(.webui-session-layout 等规则被拆进 features/*/ui/*.css 后 position 显式声明与 z-index 集合变了),修它属于 shell 布局区的合同维护,不在本 PR 范围。

第二层:webui-shell 与 workspace-tab-strip 各一条 —— 渲染出了本仓库不存在的代码

这两条只在本 PR 的 CI 上出现(本地同代码、同 C-locale、同 vitest 配置下 126/126 全过;今天其他分支的失败名单里也没有它们)。失败详情很反常:

  • workspace-tab-strip 期待本仓库手绘的 <circle cx="8" cy="8" r="5.2">(shell/icons.tsx 的 WebuiIconGlobe),CI 实际渲染的是 class="tabler-icon tabler-icon-world"(@tabler/icons-react 的产物,viewBox 0 0 24 24、纯 path)。
  • webui-shell 同样:控件按钮被渲染成 tabler-icon-browser / tabler-icon-layout-sidebar-right-collapse,其中一个按钮的 aria-label="浏览器"。

全仓检索(src + test + 全部 package.json + pnpm-lock.yaml)对 tabler 零命中——本仓库不存在 tabler 依赖,也不存在 aria-label="浏览器" 的按钮;WorkspacePanelControls.tsx 的按钮是「工作区文件侧栏」(WebuiIconFolder/WebuiIconSidebarToggle)。同一 job 的日志里,我的 22 条 markdown-mermaid 单测在跑(证明检出的是本分支代码),所以这两条失败渲染的标记不可能来自本分支的检出代码。加上今天不同分支的 webui 门在不同用例上轮流挂(04:41 是 workspace-directory-browser,07:16 起是 css-structure 两条),怀疑 runner 侧工作区污染或缓存串扰。我没有 runner 权限,无法进一步证实;建议管理员看一眼 runner 的 workspace 清理。

本地全量证据(同检出代码)

  • 新单测 22/22、依赖门 117/117;全量单测仅 css-structure 两条红(与 origin/webui 相同)
  • 浏览器套件 151 passed / 7 skipped(真库渲染 SVG、失败回退、主题跟随、宽图、半截围栏、斜杠面板 chunk 回归)
  • typecheck client+test 0 错误;入口包 4,456,586 B(比 HEAD 小 190KB),mermaid 全在 109 个懒加载 chunk

@modacker modacker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这次 CI 红也不是这条 PR 的问题,和 #222 是同一批上游改动带来的。判定以 job 日志为准(Verification failed at: test:webui),6 条失败分成两半。

一、4 条是 webui 主线自己红的,#225 已修

  • packages/webui/test/unit/webui-shell.test.ts —— 断言钉死了 <circle cx="8" cy="8" r="5.2",上游把浏览器按钮的图标换成了 tabler 图标;
  • packages/webui/test/unit/webui-w0-css-structure.test.ts 两条 —— 上游新增了 .webui-workspace-panel-resize-handle { z-index: 5 }(那条 z-index 清单里没有 5),又把三个滚动区的滚动条规则合并成了一条分组选择器(规格的查法只认「一行只写一个类名」的规则,于是查不到这条、属性读成 absent);
  • packages/webui/test/unit/workspace-tab-strip.test.tsx —— 同样钉死了 <circle,上游换成了 tabler-icon-world。

证据:把代码切到完全不含这条 PR 任何改动的上游 base(fa96ec09)本地跑,同样这 4 条挂;这条 PR 的 head 加上 #225 的修复就全过。修复在 #225(base webui),只改测试文件、不动产品代码。

二、另外 2 条跟这条 PR 的改动无关

packages/webui/test/integration/browser-panel-live-navigation.test.ts 的两条,报 The browser page has been replaced; open the tab again.(browser_target_stale)。证据:

  • 这条 PR 的改动一共 14 个文件(merge-base 2fdf960f → head 2190ae15 的三点 diff),没有一行碰 packages/webui/src/runtime、packages/webui/test/integration、packages/webui/src/shared、packages/webui/src/server;
  • 这条测试直接用 runtime 源码驱动真实 Chromium,不经过任何构建产物,所以这条 PR 改的 code splitting、新增的 mermaid 依赖都影响不到它;
  • 同一份 runtime 代码在 CI 上过了 3 次(主线 3 次 + #225 1 次),只有这次挂;本地(这条 PR 的 head、纯净 base)单跑和全量并行都过,复现不出来。

形状上是慢机上的时序问题:页面还在加载时会被观察成「被替换的页面」,运行时把 target 的 generation 顶一次,于是刚 open 拿到的引用变旧;而测试里的 waitFor 碰到这个错是直接抛、不重试。我会另开一个 PR 让这条测试能容忍这一下(重试 + 重新取当前 target),同样只动测试文件。

另外澄清一下:workspace-browser-tab-acceptance.test.ts 在这次 CI 上是通过的(14 条全过),报错的只有上面那个文件的 2 条。

结论:这次红不在这条 PR 身上。#225 合并后主线那 4 条红会转绿;剩下这 2 条时序问题由另一个 PR 处理。

@stevenjj33
stevenjj33 force-pushed the fix/webui-mermaid-fence-v2 branch from 2190ae1 to 44d7f80 Compare October 22, 2026 11:09

@modacker modacker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

补一条更正,也补上另一半。

上一条评论我写「workspace-browser-tab-acceptance.test.ts 在这次 CI 上是通过的(14 条全过)」——那说的是 ubuntu verify 那个 job。同一条 PR 的另一个 job(CLI release 的 build)里,这个文件挂了 1 条:case「a person types into their own tab while the Agent works」,报

WebuiBrowserOperationError: The browser target has been replaced; the input was not delivered.(browser-service.ts:848,输入路径)

这是同一条竞态的另一半:

  • 读路径那半(browser-panel-live-navigation.test.ts,报的是 "The browser page has been replaced; open the tab again.")已经在 #226 里修了 —— 读和命令都改成走运行时当前持有的 target,waitFor 遇到 stale 引用再给一次机会。
  • 输入路径这半我另开了 #227:那个文件自己的读路径早就用 currentIdentity 取当前 target 了,只有输入还绑在 open 返回的那个旧引用上,所以输入会被运行时当成「已被替换的目标」拒掉。

两半都是同一个验证方式:在本地把竞态强制复现出来(刚 open 完立刻 reload 一次、等 generation 顶上去),原版会挂且报的就是 CI 那句,修完同一个注入下全过。

另外你补的那个「native acceptance host 服务 split bundle 的 chunk」是这份 PR 自己的事 —— 拆分包让 native host 拷进临时目录的只有入口文件、缺 assets/,那个目录没有就根本起不来。这个我这边没有异议。

@stevenjj33

Copy link
Copy Markdown
Collaborator Author

回应两条评审 + 剩下那个 build check 的取证

两条评论都收到:host 拷 assets/ 确实是本 PR 自己的配套(splitting 让临时目录里的入口缺 chunk 就起不来),已作为独立提交在头里(d9f55870);当前 head 已 rebase 到 ea62949(含 #225),Source verification 三平台 + performance 全绿。两条评论没有对本 PR 提出需要修改的点,我这边无改动待办。

关于还红着的 build(CLI release 工作流)——不是这条 PR 的代码问题

这个 check 之所以出现在本 PR,是路径过滤器里的 pnpm-lock.yaml:本 PR 加了 mermaid 依赖动了 lockfile,才触发了这个工作流(#222/#223 没动 lockfile,所以没有这个 check)。

它红在 package-cli-release.mjs:79 的 「Packaging requires a clean committed working tree」——build job 在 ubuntu 上跑完默认 pnpm verify(21 门全过,含 test:webui-browser: PASS (247.5s))之后,打包前发现被跟踪文件被改动了。取证:

  1. 该工作流近期没有任何 PR 通过过:往前数 16+ 连败(10-20 至今,横跨 feat/webui-browser-use 和本分支,原因各异:curl 下载失败、test:webui-browser、这次的 clean-tree)。PR 事件上它从来不是绿的。
  2. 本地复现不出:同一条默认 pnpm verify 在我这里(darwin,浏览器门按平台跳过)跑完 git status --porcelain --untracked-files=no 为空——脏发生在 Linux 特有那段(最可能是 test:webui-browser 的某个环节写了被跟踪文件,具体哪个文件日志里没有)。
  3. 本 PR 的 14 个文件的写入点都查过,没有一个会在 verify 过程中写被跟踪文件。

merge 依据的 Source verification 是全绿的;这个 release 工作流在 PR 上的健康度建议管理员看一下(要么修掉 verify 期间的 tracked-file 写入,要么 PR 事件不跑打包步)。需要我配合在 CI 上打印脏文件清单的话说一声,我可以推一个临时诊断提交。

Resubmission of #125 at the relocated paths, with the review's defects
addressed.

The acceptance table carried a "markdown-图表(mmd) 通过" row while the
source had zero Mermaid support: a ```mermaid fence fell through to the
plain code path, so a model-drawn flowchart arrived as an unreadable blob
of text. The fence branch sits in markdown.public.tsx between the math
and generic code branches and fires only for the mermaid/mmd language
words; every other fence keeps its untouched path.

Three defects from the review, fixed here:

- Theme. Mermaid bakes its palette into the SVG it returns, so a diagram
  drawn with the light palette is a light diagram even on a dark page.
  The renderer now draws with the app's own theme, read through the
  injected DOM adapter (readDocumentTheme) — the dependency rules
  confine the ambient document to the platform layer, so the component
  must not touch it. The library is re-initialised when the theme
  changes, so the next diagram follows a switch.

- Untrusted content. The diagram body is model output and the rendered
  SVG is injected with dangerouslySetInnerHTML, so the SVG is filtered
  before it reaches the DOM: scripts, event handlers, script-ish URLs
  and foreign documents are removed, and the drawing itself is left
  alone. Mermaid's own strict mode stays the primary control; the
  filter is the second layer. htmlLabels is off, so the diagram is pure
  SVG and the labels are escaped text rather than HTML in a
  foreignObject.

- Tests. The unit suite mocks the library (Node has no DOM for it), so
  the browser cases run the real one: a diagram renders to SVG, an
  invalid diagram falls back to the plain fence, the diagram follows
  the app's theme, a wide diagram fits its container, and a
  half-streamed fence falls back instead of breaking the transcript.
  The unit suite covers the sanitizer and the theme read.

Build: the client bundle is split, so a dynamic import is a chunk
instead of inlined source. Measured on the built artifact: the entry
goes 4,647,161 B to 4,456,586 B (shared code moves into chunks) and
mermaid's ~12 MB lands in 109 lazy chunks under assets/. The browser
test server now serves that prefix, matching the loopback service's
static resolver; the composer's slash palette — already a dynamic
import — is chunked too, and its browser case covers the regression.
The native-browser-host copied index.html, client.js and styles.css into
its temp clientDir and nothing else. The split bundle's entry imports its
chunks from /assets/ — a copy without the directory cannot boot at all,
so the PTY acceptance test died waiting for #webui-root (reproduced on
the ubuntu CI twice; the app boots the moment the directory is copied).

The directory is now copied wholesale, matching what the loopback
resolver serves and the packaged release ships.

@modacker modacker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

整体看下来这版是扎实的:四个「刻意选择」(动态 import、imperative render、纯展示块 + 效果壳、渲染后净化)都在注释里说清了理由,失败一律回落到普通围栏,单元与浏览器两层的用例也把这个契约覆盖住了。新增文件和依赖都登记了(release/public-source.json、test/vitest-suites.json、packages/webui/package.json 的 mermaid 固定版本),mermaid 只打进客户端 chunk、没有进发布包的依赖(发布包仍只有 better-sqlite3 和 node-pty),这点是对的。

下面几条是想请你看一眼的,按重要性排:

1. 切主题时,已经画好的图不会重画(webui-mermaid.tsx:280-291)

主题是在 effect 里读的,而依赖只有 [dom, text](dom 来自 context,跨渲染是稳定的)。所以用户把主题切成深色时,已经渲染出来的那张图还是浅色调色板 —— mermaid 把调色板烤进 SVG,深色页面上就成了一张浅色的图。你在 loadMermaid 的注释里写的是「下一个图会跟着换」,这个范围本身是自洽的,但读者容易以为当前这张也会跟着换,而且你自己在别处描述过症状(「a diagram drawn with the light palette is a light diagram even on a dark page」)。

浏览器那条 a diagram follows the app's theme 是 page.reload() 之后再比对调色板的,所以覆盖不到这个路径。要么把主题也变成依赖(需要一个能订阅主题变化的地方),要么在注释里明确「已经画好的图不会重画」。

2. theme 选项的注释和代码对不上(webui-mermaid.tsx:175 vs :190)

注释说 “defaults to the app's current theme”,代码是 options?.theme ?? "light"。这个函数是纯函数,拿不到应用的主题,实际默认值就是 light(组件里是显式传的,所以线上行为没问题)。建议把注释改成「默认 light;组件会显式传应用当前的主题」。

3. 回落到普通代码块时没有行号槽(webui-mermaid.tsx:235)

你在注释里写明「a Mermaid fence never shows line numbers in any of its states」,单测也断言了「with no gutter」,所以这是有意选的。只是提一句:一个渲染失败的 mermaid 围栏在读者眼里就是个普通代码块,而普通代码块(ts 围栏)是带行号槽的,两者看起来会不太一样。属于产品取舍,你决定就行。

4. splitting: true 是全仓产物布局的改动(scripts/build-webui.mjs、scripts/package-webui-npm.mjs)

这个开关影响的是整个客户端的产物结构,不只是 mermaid。原因(不切分会把 mermaid 和 slash palette 内联进 entry)和 resolveClientAsset 已经伺服 /assets/ 前缀这件事都对,三平台和打包门禁也都绿了,所以是安全的。只是影响面比一个 feature 大,想请你自己确认一次这是有意为之。

5. 过滤保留了 <style> 元素

净化函数去掉了 script / foreignObject / iframe / on* / javascript: URL,但 <style> 是留下来的(图里的样式需要它)。模型生成的图能不能借 mermaid 指令带进任意 CSS,取决于 strict 模式挡不挡 init 指令 —— 如果挡不住,这里是个口子。知道这是你有意留的,只是没看到你说明为什么 <style> 是安全的。

顺带澄清一件事:那两条浏览器控制台报错(session.title?.trim is not a function 和 Objects are not valid as a React child (found: object with keys {not}))不是这个 PR 引入的。它们来自 packages/webui/test/browser/settings-account-tab.spec.mjs:282 故意塞的畸形会话(title: { not: "a string" }),基线上就有,纯净 base 的浏览器套件里同样会出现。我先前说成是这个 PR 的问题,那是我的错。

结论:这条 PR 我没有阻塞意见,同意合入。等打包门禁那条(验收记录在 CI 还原)落地后它就能一起走完。

@modacker modacker closed this Oct 22, 2026
@modacker modacker reopened this Oct 22, 2026
@modacker modacker closed this Oct 22, 2026
@modacker modacker reopened this Oct 22, 2026
@modacker
modacker merged commit 63b9739 into webui Oct 22, 2026
42 of 46 checks passed
modacker added a commit that referenced this pull request Oct 23, 2026
…nder

fix(webui): Mermaid 已画图形随主题切换重画(#224 评审跟进)
@modacker
modacker deleted the fix/webui-mermaid-fence-v2 branch October 23, 2026 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants