perf(export): split the transcript renderer's embedded CSS into a versioned asset - #11485
Conversation
…sioned asset The export renderer carried the web-shell component stylesheet as a ~2.3 MB string literal, so every reader parsed and compiled 4.1 MB of JS (56% of it dead CSS) before a transcript could render. Lift that literal out at export build time into a version-pinned, SRI-protected export-transcript-document.css served from unpkg and loaded via a nonce-bearing <link>, dropping the renderer JS to ~1.83 MB. The transform is an esbuild onLoad plugin in the web-templates export build that strips the injected CSS constant from web-shell's dist/transcript.js; web-shell source and runtime behavior are untouched. The document's fail-closed load-error path is extended to the stylesheet so a missing CSS asset fails the same way as a missing renderer.
github-actions
Bot
added
the
review/self-reported
|
Re-run at your request. The last gate pass looked at Template looks good ✓ — all nine headings present, and the Before/After table carries real byte counts rather than adjectives. Problem: observed and measured, not theoretical. #11478 records the composition of the shipped asset (56% of 4.1 MB is one CSS string literal) and the budget sitting ~66 KB under its hard cap. The numbers were independently re-measured on a real build by @wenshao in his round-2 report — Direction: aligned. Two things put this in "escalate, never auto-reject" territory rather than a clean pass: it touches the release contract (a new required publish artifact, plus a second public CDN dependency for every exported document), and the issue's own triage set conditions A–G that this PR answers unevenly — see the Approach section. Size: no core paths. Approach: the scope feels right and I could not find a materially simpler path. I wrote down what I would do before reading the diff — esbuild No drive-by refactors and no unrelated churn — the diff stays inside the stated goal. Where I would push back is on the issue's conditions:
Risk: no high-risk-path matches — nothing in the diff touches the paths this repo's revert history correlates with. The elevated risk here is coordination, not code, and I have left it for Stage 3: #11372 is APPROVED and open, edits the same two constants in the same file from the same base blob in the opposite direction ( Flagging these for discussion; moving on to code review. 🔍 中文说明应你的要求重跑。上一次门禁看的是 模板完整 ✓ —— 九个标题齐全，Before/After 表给的是真实字节数而不是形容词。 问题： 已观测且有实测，不是理论性问题。#11478 记录了产物的构成（4.1 MB 中 56% 是一个 CSS 字符串字面量），以及预算距硬上限只剩约 66 KB。这些数字由 @wenshao 在真实构建上独立复测过（见其第二轮报告）： 方向： 对齐。 有两点让它落在"升级给维护者、绝不自动拒绝"而不是干净通过：它触及发布契约（新增一个必须发布的产物，并且每个导出文档都多了一个公共 CDN 依赖）；同时 issue 自身的 triage 设了 A–G 条件，而本 PR 的回答并不均衡——见下方"方案"。 规模： 未触及核心路径。 方案： 范围合理，我没有找到明显更简的路径。在读 diff 之前我先写下了自己的做法——在导出构建边界用 esbuild 没有顺手重构，也没有无关改动——diff 始终待在既定目标内。 我要提出异议的是 issue 的那几条：
风险： 无高风险路径命中——diff 没有触及本仓库回滚历史相关联的那些路径。这里升级的风险是协同性的，不是代码性的，我把它留到 Stage 3：#11372 目前是 APPROVED 且开着，从同一个 base blob 出发、在同一个文件里以相反方向改同样那两个常量（ 先把这些提出来讨论；已进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Code review at Code reviewNo Critical findings of my own at this head. I read the build plugin, the digest and URL derivation, the document template, the renderer's two guard sites, the formatter's nonce substitution, and all three packaging scripts. Specifically:
Both round-1 Criticals are fixed, and I verified that by reading the code at this head rather than taking the reports for it. R1-1's filter is now Two of my own notes, neither blocking:
Suggestions I am explicitly deferring rather than asking for. This PR has been through roughly five review rounds, so per this repo's own guidance I am landing only Critical fixes and recording the rest here so nothing is silently dropped. All of the following are Suggestion-level, all were measured by @wenshao, and none is a defect in the shipped code path:
The delegation runbook is the one I would fold in before merge rather than after: it is the documented escape hatch for the 404-until-release window this PR introduces, and as written it fails. Why the head latch existsThe ordering hazard that was round 1's second Critical, and the two disjoint windows the head latch and the body listener now cover: sequenceDiagram
participant P1 as Browser parser
participant P2 as head latch script
participant P3 as stylesheet link
participant P4 as body listener script
participant P5 as renderer script
participant P6 as document main module
P1->>P2: parse head script, register capture listener
P1->>P3: parse link, start CSS fetch (parser blocks here)
P3-->>P2: error event (capture phase, does not bubble)
P2->>P2: record the failure flag (body does not exist yet)
P1->>P4: parse body script
P4->>P4: register own listener for both asset ids
P4->>P4: read the flag, show load error, mark body as error
P1->>P5: parse renderer script (only after the sheet settles)
P5->>P6: module executes
P6->>P6: refuse to mount when the marker says error
The latch catches a failure dispatched before the body script exists; the body listener catches one dispatched after. @wenshao mutated each id comparison independently and both mutations produce an unstyled transcript in a real browser, so neither half is redundant. Test evidenceThis comment carries the PR's own CI results, read from the check-runs API for
The one red check is a pre-existing flake in this PR's base, already fixed on
Remedy is a rebase onto The lane that matters for this PR is green, and it ran at this head. Inside that same job, step 15 "Run transcript document browser gate" completed successfully before step 16 failed: Not verified by me, and named as such:
Third-party maintainer verification exists and is unusually strong, but it is not CI and not mine: @wenshao (admin) built both arms from source and drove a real browser against a local HTTP origin at this exact head — 241 tests green, a 12-cell fail-closed matrix closed in every failure cell, render parity against merge-base at 0 differing pixels in both themes, and a mutation matrix per finding (round 1, round 2). His measured time-to-render is median 326 ms against 383 ms on merge-base. I am citing that as his measurement, not as evidence I reproduced. Sandboxed verification would settle the remaining half: 中文说明在 代码审查在这个 head 上我没有自己的 Critical 发现。 我读了构建插件、摘要与 URL 推导、文档模板、渲染器的两处守卫、formatter 的 nonce 替换，以及三个打包脚本。具体来说：
round 1 的两个 Critical 都已修复，而且我是通过在这个 head 上读代码确认的，不是采信报告。 R1-1 的 filter 现在是独立模块里的 我自己的两条备注，都不阻断：
我明确延后、而不是现在要求的建议。 本 PR 已经过约五轮评审，因此按本仓库自己的指引，我只落 Critical 修复，并把其余记录在此以免被悄悄丢掉。以下全部是建议级、全部由 @wenshao 实测过、且都不是已发布代码路径上的缺陷：
其中委派 runbook 是我建议在合入前而不是合入后处理的一条：它是本 PR 引入的"发布前 404 窗口"的既定应急手段，而按现状它是坏的。 head latch 为什么存在上面那张时序图画的正是 round 1 第二个 Critical 的时序陷阱，以及 head latch 与 body 监听器现在各自覆盖的两个互不相交的时间窗。latch 接住在 body 脚本存在之前派发的失败，body 监听器接住之后派发的失败。@wenshao 分别对两处 id 比对做了变异，两个变异在真实浏览器里都会产生无样式 transcript，所以两半都不是冗余的。 测试证据本条评论携带的是 PR 自己的 CI 结果，从 （CI 表格见上方英文部分，机器可读区域标记内。） 唯一的红检查是本 PR base 上的既有 flaky，且已在
解决办法是 rebase 到 对本 PR 真正关键的那条流水线是绿的，而且是在这个 head 上跑的。 在同一个 job 内部，第 15 步 "Run transcript document browser gate" 在第 16 步失败之前已成功完成： 我未验证、并如实标注的部分：
存在第三方的维护者验证，而且强度罕见地高，但它不是 CI、也不是我做的：@wenshao（admin）从源码构建了两条基线，并在这个 head 上对真实本地 HTTP 源站驱动了真实浏览器——241 条测试全绿、12 格 fail-closed 矩阵在每个故障格都是 closed、与 merge-base 的渲染差异在两种主题下均为 0 像素、以及逐条发现的变异矩阵（第一轮、第二轮）。他实测的渲染耗时为中位数 326 ms，对 merge-base 的 383 ms。我引用的是他的测量，不是我自己复现的证据。 沙箱验证可以补上剩下的一半： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — I found no code defect at this head and I would merge the implementation; what is outstanding is a rebase and a sequencing decision, not a fix. Stepping back. I wrote my own approach down before reading the diff, and yours matched it point for point — export-build-boundary strip, fail-closed on shape drift, I also want to correct my own last pass, because it is still sitting in this thread and it was wrong. I asked for a What I am not going to wave through is the evidence state, and it is not about code quality. The branch needs a rebase, and that is worth more than an approval from me right now. Related, and easy to miss: the #11372 needs a decision before either PR merges, and only you can make it. It is Condition A is a maintainer call, not a gate call. You took the ratchet from covering 4.1 MB of render-blocking bytes to covering 1.83 MB of them, and the Everything else I found is Suggestion-level and I am deferring it rather than asking for another round — this PR has been through roughly five, and this repo's own guidance at that point is to land Critical fixes only and record the rest. The list is in Stage 2 with the measurements behind each item. The one I would genuinely fold in now is the delegation runbook, because its copy-pasteable two-knob command now throws and that command is the documented escape hatch for the 404-until-release window this PR introduces. ⏸️ Deferring — no approval and no request for changes. @yiliang114, both blocking items are yours and neither is a code change: rebase onto I expect to approve on the re-run once the head carries the rebase and #11372 has an answer. Nothing in the implementation is holding this back. 中文说明Confidence: 3/5 —— 在这个 head 上我没有找到代码缺陷，实现本身我会合；尚未解决的是 rebase 与先后顺序的决定，不是修复。 退一步看。我在读 diff 之前先写下了自己的做法，而你的方案与之逐点吻合——在导出构建边界剥离、形状漂移时 fail-closed、对实际发布字节做 我还要更正我上一轮的说法，因为它还留在这个 thread 里，而它是错的。我当时要求在挂载前判一次 我不打算放行的是证据状态，而这与代码质量无关。 这个分支需要 rebase，而这比我此刻给你一个批准更有价值。 相关、且容易被忽略的一点： 本 PR 上目前挂着的 #11372 需要在任一 PR 合入之前有决定，而这个决定只有你能做。 它现在是 条件 A 是维护者的决定，不是门禁的决定。 你把棘轮从"覆盖 4.1 MB 渲染阻塞字节"改成"覆盖其中 1.83 MB"，而 我找到的其余一切都是建议级，我选择延后而不是再要一轮——本 PR 已经过约五轮，而本仓库在那个节点上的指引就是只落 Critical 修复、其余记录在案。清单在 Stage 2，每一条后面都附了实测。我唯一真心建议现在就带上的，是委派 runbook：那条可复制的两开关命令现在会抛错，而它正是本 PR 引入的"发布前 404 窗口"的既定应急手段。 ⏸️ 暂缓 —— 不批准，也不请求变更。 @yiliang114，两个阻断项都是你的，而且都不是代码改动：rebase 到 一旦 head 带上 rebase、且 #11372 有了答案，我预期会在重跑时批准。实现本身没有任何东西在拖住它。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
真实数据验证（优化差异）用仓库里真实的 4 条 ChatRecord fixture 走了一遍完整 体积（
即：总线上字节基本不变，但浏览器「解析+编译」的关键路径从 4.1 MB 降到 1.83 MB（gzip 1.5 MB → 0.5 MB）。 真实导出产物（
无头 Chromium 打开真实导出：
CI上一轮 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-3 byte ratchet no longer covers the lifted CSS asset — already reported (comment 5603265110, stage-2 item 1; also comments 5603265631 and 5603264729)
- R1-20 delegation docblock and runbook still describe a two-variable contract — already reported (comment 5603265110, stage-2 item 3; also comment 5603265631)
Unresolved, please confirm:
- [Critical] stage-3 triage blocker (comment 5603265631): whether open PR #11372 or this PR lands first — verified #11372 is OPEN and edits the same packages/web-templates/src/export-html/build.mjs (+6/-8) in the opposite direction, but the sequencing d…
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 6 by the review time budget.
Test Plan (not a blocker): 6 passed — this review observed 29641 passed.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.
未审查：反向审计——评审时间预算不足，未能开始第 6 轮。
Test Plan（非阻断）：6 passed — this review observed 29641 passed。
— qwen3.8-max via Qwen Code /review (v0.23.2)
esbuild hands plugin callbacks the platform-native absolute path, so the extract-transcript-css `onLoad` filter never matched on Windows: the callback did not run, `extractedTranscriptCss.css` stayed undefined, and the mandatory extraction guard below aborted the build. That build is not platform-gated — `scripts/prepare.js` runs it from `prepare`, so `npm ci` itself would fail on every Windows contributor and on the windows-latest legs of test_windows and desktop-release. Widen the separator to `[\\/]`, keeping the `transcript\.js$` tail so the barred `web-shell/dist/index.js` package root still does not match. The filter moves to transcript-css-entry.mjs because build.mjs is a top-level-await script with no harness — the same reason scripts/sdk-node-exporter-stub.js exists — so scripts/tests/transcript-css-entry-filter.test.js can pin both separators. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtufhms2u4
The `<link id="transcript-stylesheet">` sits in `<head>` while the `window` error listener that catches its failure is registered by an inline script in `<body>`. Chromium parser-blocks that script on the pending stylesheet, so when the CSS failure settles first the error event is dispatched with no listener to receive it: nothing marks the render as failed, both renderComplete guards in document-main.tsx pass, React mounts the transcript without any of the component CSS, and the requestAnimationFrame stamps `data-render-complete="true"`. The reviewer measured this fail-open above roughly 2.1 MB of document HTML (272 of the 1,000 permitted blocks) for a 404, an SRI rejection, a truncated body and a destroyed socket alike, and fail-closed for a *late* failure — so size, not failure kind, decides it. Latch the failure in `<head>` before the `<link>` is parsed and act on the latch from the existing body IIFE. The head script only records: `showLoadError()` writes `document.body.dataset` and `#app`, neither of which exists while the parser is still in `<head>`. It carries `nonce="__EXPORT_NONCE__"` because the document CSP allows no inline script, which is safe — `formatters/html.ts:53` replaces every occurrence. The listener is capture-phase because resource error events do not bubble. Not the `link.sheet === null` variant: the reviewer measured `sheet` non-null for a 404, a truncated body and a destroyed socket, so it only detects SRI rejection. scripts/tests/export-transcript-document-template.test.js pins the position, the nonce, the capture phase and the record-only shape; all five cases go red against the unpatched template. The behavioural witness (real Chromium, large document, instant CSS abort) belongs to the playwright transcript gate, which is out of budget on this host. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtufhms2u4
The bundle copy became all-or-nothing over two artifacts but its `else` warning still named only the renderer, so the one new way to reach that branch — a tree built before the split, then `npm run bundle`d without rebuilding web-templates, which has the JS and no CSS — told the operator to go looking for a `export-transcript-document.js` that was sitting right there, and silently discarded it. List the paths that are actually absent, matching the sibling Web Shell warning twenty lines above. Stays warn-and-skip: prepare-package.js is the release gate. Also pin that release gate. Every fixture that reached `preparePackage` staged `dist/export-transcript-document.css` unconditionally, so deleting the new required-path entry left the whole test:scripts lane green; a release built with `npm ci --ignore-scripts` would then publish documents whose stylesheet 404s on unpkg for that version. `verifyBundleArtifacts` reports through console.error + process.exit(1) rather than a throw, so the new case stubs exit instead of copying the audio-capture sibling's `toThrow` idiom. Both cases were flip-checked: restoring the old warning text, and deleting the CSS line from prepare-package.js, each turn their case red. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtufhms2u4
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] Blocking finding(s) follow.
Partially reviewed — gaps disclosed. Suggestions are inline.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-3 byte ratchet no longer covers the lifted CSS asset (build.mjs:50-51) — already reported (comment 5603265110, stage-2 item 1; also comments 5603265631 and 5603264729)
- R1-20 delegation docblock and runbook still describe a two-variable contract (build.mjs:194-201, docs/verification/export-renderer-delegation-mermaid/README.md:106-107) — already reported (comment 5603265110, stage-2 item 3; also…
- docs/users/features/commands.md:39 still describes a single version-pinned unpkg asset — already reported (comment 5603265110, stage-2 item 4; issue #11478 condition E)
- docs/verification/export-html-runtime-size/README.md:268 still quotes the log line this PR removes — already reported (comment 5603265110, stage-2 item 4)
Unresolved, please confirm:
- [Critical] stage-3 triage blocker (comment 5603265631) — whether open PR #11372 or this PR lands first. Verified #11372 is still OPEN and still edits only packages/web-templates/src/export-html/build.mjs (+6/-8) in the opposite direction, so the coord…
Not reviewed: build-and-test on Windows and macOS — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) are merge_group/schedule-gated and report skipped on this PR, and no suite ran on either host here. This is the lane class that would have executed the two new scripts/tests files on a non-POSIX filesystem, which is what round-1 Critical R1-1 turned on; the separator fix is now pinned by scripts/tests/transcript-css-entry-filter.test.js, which scripts/tests/vitest.config.ts's win32 exclude list does not name, and .gitattributes sets * text=auto eol=lf, but neither was observed running on those hosts..
Test Plan (not a blocker): 6 passed — this review observed 29640 passed.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/web-templates/src/export-html/build.mjs:366 — [probe] Build identity no longer commits to the pinned stylesheetpackages/web-templates/src/export-html/build.mjs:229 — [probe] New delegated-CSS validation branches have no testintegration-tests/chat-transcript-document.test.ts:718 — [probe] Third copy of the probe envelope and fail-closed body
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：build-and-test on Windows and macOS — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) are merge_group/schedule-gated and report skipped on this PR, and no suite ran on either host here. This is the lane class that would have executed the two new scripts/tests files on a non-POSIX filesystem, which is what round-1 Critical R1-1 turned on; the separator fix is now pinned by scripts/tests/transcript-css-entry-filter.test.js, which scripts/tests/vitest.config.ts's win32 exclude list does not name, and .gitattributes sets * text=auto eol=lf, but neither was observed running on those hosts..
Test Plan（非阻断）：6 passed — this review observed 29640 passed。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.2)
There was a problem hiding this comment.
Critical-only review at head d54fcd0f. Approving: both Criticals reported on this PR are verifiably fixed in the code at this head, and a scan of the production surface found no provable Critical.
Previously blocking findings — fixed and re-verified
R1-1 (Windows onLoad filter aborted every export build). The filter now lives in packages/web-templates/src/export-html/transcript-css-entry.mjs as TRANSCRIPT_CSS_ENTRY_FILTER = /web-shell[\\/]dist[\\/]transcript\.js$/, so it matches the platform-native path esbuild hands plugin callbacks on either separator, and the transcript\.js$ tail is retained so the barred web-shell/dist/index.js package root still cannot match. build.mjs:7 imports it and :141 uses it as the extract-transcript-css plugin's filter, with the mandatory guard at :300-303 still throwing when extraction never ran. Extracting the matcher out of the top-level-await build script is also what makes it pinnable, and scripts/tests/transcript-css-entry-filter.test.js covers both separators plus the dist/index.js negative.
R1-2 (a fast stylesheet failure reached no listener, so an unstyled transcript stamped data-render-complete="true"). document-index.html now carries a nonce-bearing <script> in <head> immediately before the <link id="transcript-stylesheet"> that registers a capture-phase window error listener — capture is required because resource error events do not bubble — and records window.__transcriptStyleFailed = true without touching document.body, which does not exist yet. The body IIFE keeps its own listener for both asset ids and then acts on the latch (if (window.__transcriptStyleFailed) showLoadError();), so a failure that settles while the parser is still blocked on the stylesheet is recorded before the body exists and consumed the moment it does. The chain then closes: showLoadError sets document.body.dataset.renderComplete = 'error', document-main.tsx:344 refuses to mount when that marker is 'error', and :247 only stamps 'true' when it is not — so the unstyled-but-successful render the finding described is no longer reachable. The nonce is load-bearing under script-src 'nonce-…' with no 'unsafe-inline', and formatters/html.ts:50-53 guards the placeholder and replaceAlls it, so the new head script and the <link> are both nonced.
The thread stays open on the author's own disclosed verification gap (never witnessed on a real multi-megabyte document or a Windows host). I am ruling on the mechanism, which is complete in the code above: the listener now exists before the <link> is parsed, so there is no window in which the event can be dispatched unheard.
Critical-only scan of the current diff
Read the full export document template, the new entry module, the build plugin and its asset/digest derivation, the renderer's guard sites, and the formatter's placeholder substitution. No blocking defect:
- The extraction is fail-closed at three points: a missing
__qwenWebShellCssconstant throws, a moved or absent runtime-injection line throws, and a plugin that never matched throws before any asset is written — so a shape change ininjectCssModulesbreaks the build rather than shipping a renderer that still injects CSS. - The SRI digest and the published bytes cannot diverge:
documentRendererCssIntegrityissha384overextractedTranscriptCss.css(build.mjs:389-390) and the asset is written from that same string (:450-452), both utf8. - The CSS URL is derived from the same published version as the JS, and the delegate path (
rendererDelegateCssIntegrity) keeps the inter-release override symmetric with the renderer's. __DOCUMENT_RENDERER_CSS_URL__/__DOCUMENT_RENDERER_CSS_INTEGRITY__are substituted beside the pre-existing renderer replacements, so no placeholder can reach a shipped document.
CI at this head: Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox) — which runs the transcript document gate that now fulfils the stylesheet from the built asset — and the TUI parity gate are all green. The rollup reads FAILURE only because three route checks were cancelled, which is not evidence of a defect here.
The twelve remaining open threads are all Suggestion-level — test-oracle strength (link.sheet not discriminating a loaded stylesheet, order and id-literal pinning, the confounded renderer-failure case), diagnostic wording that still names only the renderer, the release-time gate covering one of the two published assets, the design docs describing the pre-fix shapes, and the 404-until-release window the description already discloses. None blocks merge; they are worth a follow-up issue so they are not lost.
Correction added after submission. One unresolved item in the round-2 review body is not a code finding and is not settled by this approval: the stage-3 triage blocker asking whether open PR #11372 or this one lands first. #11372 is still open and still edits only packages/web-templates/src/export-html/build.mjs (+6/-8) in the opposite direction, so whichever lands second needs a rebase and a re-check of the extraction plugin and its guard. That sequencing call belongs to the maintainers; this approval covers the code at this head as verified above and does not resolve it.
Local verification of #11485 — real build, real browser, real network faultsVerified at TL;DRThe mechanism is sound and I could not break it. What is left is a byte-budget decision, a doc
1. The extraction is lossless (issue raised in stage-2: "regex surgery on a minified artifact")Three independent measurements, one hash: The middle line is the one that matters: it is not a re-read of the same input, it is what Chromium 2. Render parity — 0 differing pixelsA transcript exercising headings, tables, fenced/highlighted code, inline + display KaTeX, mermaid,
3. The
|
| export size | fault | 3b63662b (pre-latch) |
d54fcd0f (head) |
|---|---|---|---|
| 10 KB | CSS 404 / reset / SRI mismatch | closed | closed |
| 0.40 MB | CSS 404 | closed | closed |
| 1.20 MB | CSS 404 | closed | closed |
| 2.41 MB | CSS 404 | closed | closed |
| 3.21 MB | CSS 404 | UNSTYLED 3/3 | closed 3/3 |
| 4.0 MB | CSS 404 | UNSTYLED 3/3 | closed 3/3 |
| 4.0 MB | connection reset | UNSTYLED 3/3 | closed 3/3 |
| 4.0 MB | SRI mismatch | UNSTYLED 3/3 | closed 3/3 |
| 4.81 / 7.22 MB | CSS 404 | UNSTYLED 3/3 | closed 3/3 |
| any | JS 404 | closed | closed |
The threshold on this machine is between 2.4 MB and 3.2 MB of exported HTML. EXPORT_TRANSCRIPT_LIMITS_V1
allows a 32 MB envelope and 1,000 blocks, so this was comfortably reachable — and the SRI-mismatch row
means a corrupted or tampered CDN response would have rendered, unstyled, rather than failing closed.
The fix is correct and worth keeping.
Two things follow from that.
- The new regression test does not pin the fix. I rebuilt the export template from
3b63662b
(pre-latch) and ran the PR's own case against it:
chat-transcript-document.test.ts -t "fails closed when the CDN stylesheet is unavailable"→ 1 passed.
The test usespage.setContent+route.abort, which resolves over CDP long after the document has
parsed, so it can only ever exercise the slow path. Nothing in the suite would notice if the<head>
latch were deleted tomorrow. A pinning test needs a real origin and a document large enough to make
the parser yield; the harness on the assets branch is ~120 lines and does exactly this. - The alternative fix suggested in triage would not have worked.
link.sheet === nullwas proposed
as a simpler guard. Measured on a real 404:link.sheet !== nullwithcssRules.length === 0. The
guard would not have fired. The latch is the right shape.
4. The byte ratchet — demonstrated, symmetrically
I padded the __qwenWebShellCss literal by exactly 1,000,004 bytes on both arms and rebuilt:
| merge-base | this PR | |
|---|---|---|
| CSS +1 MB | ❌ build fails: Document export runtime is 5142389 bytes; expected <= 4200000 |
✅ build green, renderer JS is 1833944 bytes; component CSS … is 3302909 bytes |
The <link> is in <head> and is render-blocking — I measured this too: stall the stylesheet 2.5 s and
first-paint moves to 2,544 ms (nothing paints at all, not even the background). So a Tailwind scan
widening or a second KaTeX font format can add hundreds of KB to bytes the reader waits on, and every
build stays green. Adding a second constant pair for the CSS is ~4 lines; if the JS-only budget is the
deliberate call, it is worth making that call explicitly rather than inheriting it from the design doc.
Also worth noting for whoever re-ratchets next: the logged figure is taken before the renderer-version
placeholder substitution, so it is 3 bytes above the asset actually written (1833944 logged vs 1833941
on disk), and the ~3 KB inline document CSS that the merge-base counted is no longer in the budget at all.
5. Performance — honest numbers
Built sizes (my build; slightly above the PR body because main moved):
| merge-base | this PR | Δ | |
|---|---|---|---|
export-transcript-document.js |
4,139,386 | 1,833,941 | −55.7 % |
export-transcript-document.css |
— | 2,302,905 | new |
| total on disk | 4,139,386 | 4,136,846 | −2,540 |
| gzip −9, total | 1,538,709 | 1,536,599 | −2,110 |
| build headroom | 57,609 B under the 4,200,000 cap (and over the warning line) | JS 96,059 B under the new cap | — |
The merge-base build prints Document export runtime exceeds the 4100000-byte warning threshold today,
which corroborates the premise of #11478 independently of the author's numbers.
Time from navigation to data-render-complete (median, headless Chromium, local origin):
| scenario | merge-base | this PR | Δ |
|---|---|---|---|
| small export, unthrottled (7 runs) | 383 ms | 335 ms | −48 ms (−12.5 %), no run overlap |
| 900-block / 7.2 MB export, unthrottled (5 runs) | 1,609 ms | 1,561 ms | −48 ms (−3 %) |
| small export, 40 Mbps / 20 ms RTT (5 runs) | 1,193 ms | 1,185 ms | −8 ms |
| small export, 10 Mbps / 40 ms RTT (5 runs) | 3,591 ms | 3,585 ms | −6 ms |
So the PR body's framing is accurate — this is a JS parse/compile win — but the size of it is ~48 ms, it
is constant regardless of transcript size, and it is noise once bandwidth is the bottleneck, because
the same 4.1 MB still has to arrive and the stylesheet is render-blocking. One more thing the split does
not buy: both URLs derive from the same exportTranscriptRendererVersion.split('+')[0], so every
release invalidates both assets together — there is no differential-caching benefit to bank on.
None of this argues against merging. It argues that #11478 condition F (base64 KaTeX fonts, Tailwind
utilities from components the transcript never imports) is where the reader-visible win actually is, and
that it should become a tracked follow-up rather than be closed out by this PR.
6. Documentation regression — confirmed by execution
The runbook whose whole purpose is this recipe still documents the two-knob contract at
docs/verification/export-renderer-delegation-mermaid/README.md:106-107. Run verbatim:
merge-base : Document export delegates its renderer to …@0.23.1-preview.0/… ✅
this PR : Error: QWEN_EXPORT_RENDERER_CSS_INTEGRITY must be set together with the renderer
delegation … ❌ exit 1
Adding the third knob works (sha384-… computed over the built CSS → build succeeds), so this is a doc
fix, not a code fix. build.mjs:188-210 still says "Set both or neither"; docs/users/features/commands.md:39
still describes one pinned asset; docs/verification/export-html-runtime-size/README.md §6 still quotes
the Document export runtime is N bytes line this PR renames. All four are already reported — I am only
adding that one of them is an executable command that is now broken.
7. Packaging and release sequencing
Closed loop, verified live: copy_bundle_assets → dist/export-transcript-document.css byte-identical to
the built asset (same sha256) → prepare-package verifyBundleArtifacts hard-requires it (removing it
gives Error: Required package artifact not found: …/dist/export-transcript-document.css) → dist
package.json files carries it → standalone exclusion list carries it. The CSS-only-missing branch in
copy_bundle_assets names the missing file, as intended by d54fcd0f.
Live CDN check (this is a release-ordering note, not a defect):
https://unpkg.com/@qwen-code/qwen-code@0.23.2/export-transcript-document.js → 200, 4,136,297 bytes
https://unpkg.com/@qwen-code/qwen-code@0.23.2/export-transcript-document.css → 404
npm dist-tag latest = 0.23.2
The 4,136,297 matches the PR body's "before" figure exactly. Since the exported URLs are built from the
repo version, no export from this branch renders until a version after 0.23.2 is published carrying both
assets — expected and disclosed, but it means this must not ship in a release where only part of the
bundle chain ran.
Verdict
Technically I have nothing blocking. The extraction is lossless, rendering is pixel-identical, and the
fail-closed path is now genuinely airtight across the real failure modes — including the one the last
commit fixed, which I confirmed was a real, reachable bug rather than a theoretical one.
Before merge I would want:
- A test that pins the
<head>latch. Right now the fix's own regression test passes without the
fix. Recipe and harness are on the assets branch. - A decision on the CSS budget, made explicitly. Four lines if the answer is "guard both".
- The delegation runbook command fixed — it is copy-pasteable and it now fails.
- fix(export): restore document runtime budget headroom #11372 sequenced. It is still open, still edits the same two constants in the opposite direction,
and if this lands first its premise is gone.
And I would re-word the PR body's performance claim to match what is measurable: ~48 ms of parse/compile
on a fast link, nothing on a slow one, total bytes unchanged — with condition F tracked as the follow-up
that actually removes bytes.
Harness, full-resolution screenshots and raw probe output: https://github.com/wenshao/qwen-code/tree/assets-pr11485
中文版报告（点击展开）
#11485 本地验证报告 —— 真实构建、真实浏览器、真实网络故障
验证提交 d54fcd0f18，对照 merge-base fbb877a48e。环境：Linux、Node 22.22.2、无头 Chromium
（Playwright）。两条基线都是从源码构建的；下面所有浏览器结论都来自一个真实的本地 HTTP 源站提供真实构建产物 ——
没有 CDP 路由拦截，没有 mock，因为路由拦截恰恰会掩盖本 PR 最后一个提交所修复的那个失效模式。
结论速览
机制是成立的，我没能把它弄坏。剩下的是一个字节预算的取舍、一处文档回归、一个合入顺序问题，以及一个缺失的测试。
| ✅ | 剥离是可证明无损的 —— 抽出的 CSS 与 merge-base 上 injectCssModules 在运行时注入的内容 sha256 完全一致，而且是在浏览器里测的，不只是比对源文件。 |
| ✅ | 与 merge-base 逐像素一致 —— 0 个差异像素，深色与浅色主题都是。 |
| ✅ | 我能构造的每一种真实故障都 fail-closed：HTTP 404、连接重置、SRI 不匹配、JS 404 —— 包括数 MB 的大导出。 |
| ✅ | 打包链路端到端闭合，两条失败分支都验证过。PR 自带的测试在本地全绿（6 + 76 + 158），当前 head 的 CI 全绿，含 web-shell E2E Smoke。 |
| 🔴 | <head> latch（c57203fc）是承重的 —— 我在 3b63662b 上复现了它修复的那个 bug，而PR 新增的回归测试在没有该修复时同样通过。 |
| 🟠 | 字节棘轮不再覆盖渲染阻塞载荷的 56%。这是实测而非论证：CSS 加 1 MB，本 PR 构建照样全绿，merge-base 则直接构建失败。 |
| 🟠 | docs/verification/export-renderer-delegation-mermaid/README.md 里可直接复制的委派命令现在会硬抛错。已实际执行验证。 |
| 🟠 | 可测得的用户可见收益是 ~48 ms，并且在带宽受限的连接上完全消失。总字节数没有变化（−2,540）。 |
1. 抽取是无损的（对应 stage-2 的质疑："在压缩产物上做正则手术"）
三个互相独立的测量，同一个哈希：
sha256 e0e4a14164081338ff63621c15b46c31f9298f3fbe5808be2cbaf50c09cf3a8d 2,302,905 字节
├─ packages/web-shell/dist/transcript.js 中 `__qwenWebShellCss` 字面量的 JSON.parse 结果
├─ 在真实浏览器中打开一个用 MERGE BASE 构建的导出文件（即今天读者拿到的东西），
│ 读出运行时注入的 <style data-qwen-web-shell="component"> 的 textContent
└─ 本 PR 产出的 export-transcript-document.css（以及打包后的 dist/ 副本）
中间那一行才是关键：它不是把同一个输入再读一遍，而是 main 上 Chromium 样式表里真正存在的东西。
CSS 也确实离开了 JS：__qwenWebShellCss 在 merge-base 的 bundle 中存在、在 head 中消失；只可能来自样式表的
KaTeX_Main 同样如此。
2. 渲染一致性 —— 0 个差异像素
一份覆盖标题、表格、带高亮的围栏代码、行内与块级 KaTeX、mermaid、任务列表、引用块、file-diff 工具卡片和
shell 工具卡片的 transcript，在两条基线上都走完整 /export html 链路并整页截图：
compare -metric AE 在深色主题下为 0，点击 Light theme 后仍为 0，输出 PNG 逐字节相同。
条件 B（层叠顺序位于内联 <style> 之后）得到了实测支持，而不只是代码审读。
3. <head> latch 是承重的 —— 而且没有测试钉住它
批准该 PR 的审查者是就"机制"下的结论，并明确留了一个口子：这个修复*"从未在真实的数 MB 文档上被观察到"*。
现在观察到了。
复现方式： 用一个真实的本地源站同时提供导出文件与两个资产，对样式表返回真实的 404。在 3b63662b
（c57203fc 之前）上，对一个大导出，<link> 的 error 事件在解析器仍被它阻塞时就被派发 —— 早于 <body>
脚本注册监听器 —— 因此没有任何人接住它，渲染器照样挂载：
面板 B 就是这个 bug：data-render-complete="true"、没有报错页，并且
getComputedStyle('.katex').fontFamily === '"Times New Roman"' —— 组件样式表根本没生效。
面板 C 是同一份文档、同一个 404 在当前 head 上的表现。
| 导出体积 | 故障 | 3b63662b（latch 前） |
d54fcd0f（head） |
|---|---|---|---|
| 10 KB | CSS 404 / 重置 / SRI 不匹配 | fail-closed | fail-closed |
| 0.40 MB | CSS 404 | fail-closed | fail-closed |
| 1.20 MB | CSS 404 | fail-closed | fail-closed |
| 2.41 MB | CSS 404 | fail-closed | fail-closed |
| 3.21 MB | CSS 404 | 无样式渲染 3/3 | fail-closed 3/3 |
| 4.0 MB | CSS 404 | 无样式渲染 3/3 | fail-closed 3/3 |
| 4.0 MB | 连接重置 | 无样式渲染 3/3 | fail-closed 3/3 |
| 4.0 MB | SRI 不匹配 | 无样式渲染 3/3 | fail-closed 3/3 |
| 4.81 / 7.22 MB | CSS 404 | 无样式渲染 3/3 | fail-closed 3/3 |
| 任意 | JS 404 | fail-closed | fail-closed |
本机的翻转阈值在导出 HTML 的 2.4 MB 与 3.2 MB 之间。EXPORT_TRANSCRIPT_LIMITS_V1 允许 32 MB 信封、
1,000 个 block，所以这个区间是完全够得着的 —— 而 SRI 不匹配那一行意味着：一个被损坏或被篡改的 CDN 响应，
在修复前会以无样式的方式渲染出来，而不是 fail-closed。这个修复是对的，值得保留。
由此引出两点。
- 新增的回归测试并没有钉住这个修复。 我用
3b63662b（latch 前）的模板重新构建后，跑 PR 自己的用例：
chat-transcript-document.test.ts -t "fails closed when the CDN stylesheet is unavailable"→ 1 passed。
该用例用的是page.setContent+route.abort，中止经 CDP 回来时文档早已解析完毕，所以它只可能覆盖慢路径。
如果明天有人删掉<head>latch，整个测试套件不会有任何反应。要钉住它，需要真实源站 + 一份大到能让解析器
让出主线程的文档；assets 分支上的 harness 约 120 行，做的正是这件事。 - triage 里建议的替代修复不会奏效。 当时建议用
link.sheet === null作为更简单的判据。在真实 404 上实测：
link.sheet !== null且cssRules.length === 0，这个判据根本不会触发。latch 的形状才是对的。
4. 字节棘轮 —— 对称的实测反证
我在两条基线上都把 __qwenWebShellCss 字面量精确加长 1,000,004 字节后重新构建：
| merge-base | 本 PR | |
|---|---|---|
| CSS +1 MB | ❌ 构建失败：Document export runtime is 5142389 bytes; expected <= 4200000 |
✅ 构建全绿，renderer JS is 1833944 bytes; component CSS … is 3302909 bytes |
<link> 位于 <head>，是渲染阻塞的 —— 这一点我也测了：把样式表拖延 2.5 s，first-paint 就变成
2,544 ms（期间什么都不绘制，连背景都没有）。也就是说，Tailwind 扫描范围变宽、或 KaTeX 再内联一种字体格式，
都可能给读者必须等待的字节加上数百 KB，而每次构建仍然是绿的。给 CSS 再加一对常量大约 4 行；如果"只对 JS 设预算"
是有意为之，那也值得显式地做出并承担这个决定，而不是从设计文档里继承下来。
另外提醒下一个重新收紧棘轮的人：日志里的数字是在渲染器版本占位符替换之前取的，因此比实际写盘的资产大 3 字节
（日志 1833944 vs 磁盘 1833941）；而 merge-base 曾计入的那约 3 KB 内联文档 CSS，现在完全不在预算之内了。
5. 性能 —— 诚实的数字
构建体积（我的构建，略高于 PR 正文，因为 main 已经前进）：
| merge-base | 本 PR | Δ | |
|---|---|---|---|
export-transcript-document.js |
4,139,386 | 1,833,941 | −55.7 % |
export-transcript-document.css |
— | 2,302,905 | 新增 |
| 磁盘总计 | 4,139,386 | 4,136,846 | −2,540 |
| gzip −9 总计 | 1,538,709 | 1,536,599 | −2,110 |
| 构建余量 | 距 4,200,000 上限仅 57,609 B，且已超过警告线 | JS 距新上限 96,059 B | — |
merge-base 的构建今天就会打印 Document export runtime exceeds the 4100000-byte warning threshold，
这独立于作者给出的数字，佐证了 #11478 的前提。
从导航到 data-render-complete 的耗时（中位数，无头 Chromium，本地源站）：
| 场景 | merge-base | 本 PR | Δ |
|---|---|---|---|
| 小导出，不限速（7 次） | 383 ms | 335 ms | −48 ms（−12.5 %），两组区间不重叠 |
| 900 block / 7.2 MB 导出，不限速（5 次） | 1,609 ms | 1,561 ms | −48 ms（−3 %） |
| 小导出，40 Mbps / 20 ms RTT（5 次） | 1,193 ms | 1,185 ms | −8 ms |
| 小导出，10 Mbps / 40 ms RTT（5 次） | 3,591 ms | 3,585 ms | −6 ms |
所以 PR 正文的定性是准确的 —— 这是一次 JS 解析/编译的收益 —— 但它的量级是 ~48 ms，且与 transcript 大小无关，
一旦带宽成为瓶颈就淹没在噪声里，因为同样的 4.1 MB 还是要传完，而且样式表是渲染阻塞的。还有一点这次拆分
没有买到：两个 URL 都取自同一个 exportTranscriptRendererVersion.split('+')[0]，因此每次发版都会同时让两个
资产失效 —— 不存在可以指望的差分缓存收益。
这些都不构成反对合入的理由。它们说明的是：#11478 的条件 F（base64 内联的 KaTeX 字体、transcript 根本不会引入的
组件所产生的 Tailwind 工具类）才是读者可感知收益的真正来源，应该变成一个有跟踪的后续 issue，而不是被本 PR 顺手
关掉。
6. 文档回归 —— 用执行确认
那份存在意义就是这个配方的 runbook，在
docs/verification/export-renderer-delegation-mermaid/README.md:106-107 仍然是两开关形态。原样执行：
merge-base ：Document export delegates its renderer to …@0.23.1-preview.0/… ✅
本 PR ：Error: QWEN_EXPORT_RENDERER_CSS_INTEGRITY must be set together with the renderer
delegation … ❌ exit 1
补上第三个开关就能跑通（用构建出的 CSS 计算 sha384-… → 构建成功），所以这是文档要改，不是代码要改。
build.mjs:188-210 仍写着 "Set both or neither"；docs/users/features/commands.md:39 仍描述只有一个版本固定资产；
docs/verification/export-html-runtime-size/README.md §6 仍引用本 PR 改名掉的 Document export runtime is N bytes。
这四处此前都已被报告过 —— 我只补充一点：其中一条是可执行的命令，现在是坏的。
7. 打包与发布顺序
链路闭合，已实测：copy_bundle_assets → dist/export-transcript-document.css 与构建产物逐字节一致（同一 sha256）
→ prepare-package 的 verifyBundleArtifacts 硬性要求它（删掉后报
Error: Required package artifact not found: …/dist/export-transcript-document.css）→ dist package.json 的
files 带上了它 → standalone 排除列表也带上了它。copy_bundle_assets 中"只缺 CSS"的分支会准确点名缺失文件，
符合 d54fcd0f 的意图。
线上 CDN 实测（这是发布顺序提醒，不是缺陷）：
https://unpkg.com/@qwen-code/qwen-code@0.23.2/export-transcript-document.js → 200，4,136,297 字节
https://unpkg.com/@qwen-code/qwen-code@0.23.2/export-transcript-document.css → 404
npm dist-tag latest = 0.23.2
其中 4,136,297 与 PR 正文的 "before" 数字完全吻合。由于导出文件里的 URL 由仓库版本号推导，在发布一个晚于 0.23.2
且同时携带两个资产的版本之前，本分支产出的导出都渲染不出来 —— 这在预期之内也已披露，但也意味着它不能被夹带进
一次只跑了部分打包链路的发布。
结论
技术上我没有阻断项。抽取无损、渲染逐像素一致、fail-closed 路径在真实故障下确实是密封的 —— 包括最后一个提交
修掉的那个，我确认了它是一个真实可达的 bug，而不是理论风险。
合入前我希望看到：
- 一个钉住
<head>latch 的测试。目前这个修复自己的回归测试在没有修复时也能通过。配方和 harness 在
assets 分支上。 - 对 CSS 预算做一次显式决定。如果答案是"两个都守"，大约 4 行。
- 修好委派 runbook 里的命令 —— 它是可直接复制的，而现在是坏的。
- 把 fix(export): restore document runtime budget headroom #11372 排好顺序。它仍然开着、仍然反方向改同样那两个常量；本 PR 先合，它的立论就没了。
另外建议把 PR 正文的性能表述改成可测量的口径：快连接下约 48 ms 的解析/编译收益、慢连接下没有收益、总字节不变 ——
并把条件 F 作为真正"删字节"的后续跟踪项。
Harness、原分辨率截图与原始探针输出：https://github.com/wenshao/qwen-code/tree/assets-pr11485
…gn docs The <head> latch, the body listener and the <link> each spell 'transcript-stylesheet' independently and nothing compared them, so renaming either listener's id left the whole suite green while the latch recorded nothing - reinstating the fail-open the latch was added to close. Derive the id from the <link> and assert both listeners compare against it. Verified red under both mutations: latch id -> 'transcript-renderer' (1 failed | 5 passed), and the mirror with the body listener's id wrong and the latch intact (same). Both design docs still specified the two shapes the previous round replaced: the forward-slash-only onLoad filter that never matches on Windows, and the body-listener-only fail-closed extension. Section 1 now quotes the shipped TRANSCRIPT_CSS_ENTRY_FILTER and names transcript-css-entry.mjs, section 2 describes the <head> latch (position, nonce, capture phase, record-only), section 3 names the module-level render guard, and "Files affected" lists the three omitted files. EN and zh-CN are updated in the same commit. Also correct the shape-guard comment in build.mjs: the document nonces every <style> created through document.createElement, so the CSP would not block an un-stripped duplicate, and a 367-byte regrowth stays inside both byte budgets. That throw is the only guard on the duplicate-injection path. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtuo29vduf
Round 2 — re-verified at
|
| mutation | old test file (d54fcd0f) |
new test file (328feb43) |
browser, real origin |
|---|---|---|---|
| latch compares the wrong id | 5 passed |
1 failed | 5 passed |
4.0 MB export + instant CSS 404 → UNSTYLED (small export still closed) |
| body listener compares the wrong id | 5 passed |
1 failed | 5 passed |
small export + 404 / slow-404 / SRI mismatch → UNSTYLED; 4.0 MB export + slow-404 → UNSTYLED |
So they cover disjoint timing windows — the latch catches a failure dispatched before the body script
exists, the body listener catches one dispatched after — and neither is redundant. Worth saying plainly:
this test is the only thing pinning either, and it earns its place.
The corrected build.mjs comment is factually right. Measured in the exported document: a <style>
created through the document's createElement shim is nonced and applies (rgb(1,2,3)); an identical
<style> created so the shim never sees it is CSP-blocked with
Applying inline style violates ... 'style-src-elem 'nonce-…''. The old comment's claim that the CSP
would block an un-stripped duplicate injection was wrong, and the correction is the accurate one.
The design docs really are synced, both languages: section 1 quotes the shipped
TRANSCRIPT_CSS_ENTRY_FILTER and names transcript-css-entry.mjs, section 2 describes the <head> latch
(position, nonce, capture phase, record-only, body-side consumption), section 3 names the module-level
render guard, and "Files affected" lists the three previously omitted files.
Deferred findings — settled here, since the browser lane is out of budget on the author's host
The replies on this PR defer several findings because they need Playwright/Chromium and a build. I have
both, so I ran them. Each row is a mutation applied to the head tree, the suites re-run, and the mutant
opened in a real browser.
| finding | verdict | witness |
|---|---|---|
| R1-8 publish gate has no negative test | already fixed at head | deleting the prepare-package.js line reds package asset scripts > fails packaging when the published stylesheet is missing (1 failed | 35 passed) |
R1-7 <link> nonce not element-scoped |
confirmed — and worse than stated | deleting the <link>'s nonce leaves html.test.ts 3 passed and the scripts lane 42 passed; in a real browser the stylesheet is CSP-blocked and every export renders the load-error page |
| R1-6 renderer gate confounded | confirmed | deleting transcript-renderer from the body listener leaves fails closed when the CDN renderer is unavailable or fails integrity green, while a renderer-only 404 (CSS served fine) yields a completely blank page — data-render-complete unset, no alert, 0 chars of body text |
R1-18 two renderComplete guards, one unpinned |
settled | the module-level guard is the load-bearing one and is pinned (removing it → the new stylesheet case goes red; behaviourally it paints an unstyled transcript over the alert). The rAF guard is dead code: removing it keeps the gate at 6 passed and the document still fails closed |
| R1-13 cascade order unpinned | confirmed, zero live impact | putting the <link> before the inline <style> keeps every suite green (static 6, html 3, gate 6/6) and the render is pixel-identical (compare -metric AE = 0) |
R1-21 sheetLoaded oracle |
confirmed, with one correction | with integrity in place: 404 → sheet !== null, 0 rules; connection reset → sheet !== null; but empty and truncated read sheet === null, because SRI blocks them. Only 404 and reset defeat the oracle, not "empty, truncated, 404" |
| R1-5 release gate is JS-only | confirmed | grep -rn export-transcript-document .github/workflows/ → 2 hits, both the JS fetch and its cmp in release-vscode-companion.yml; no .css anywhere |
| R1-4 delegation unusable in this window | confirmed | unpkg: .css is 404 at 0.23.2, 0.23.1 and 0.23.1-preview.0; npm latest = 0.23.2; the runbook's two-knob command still throws |
Panels 1 and 2 are the two mutations that ship with a fully green suite. Panel 3 is the one the new
stylesheet case does catch — note that it reaches data-render-complete === 'error' and still shows the
transcript, because React replaces #app after showLoadError wrote into it, so a check that only reads
the marker would pass while the reader sees an unstyled export.
Unchanged from round 1
Sizes (4,139,386 → 1,833,941 JS + 2,302,905 CSS; total −2,540 bytes; gzip −2,110), the byte-ratchet
counterfactual (+1 MB of CSS: green here, Document export runtime is 5142389 bytes; expected <= 4200000
on merge-base — re-run at this head, still green), and the render-blocking measurement (2.5 s stylesheet
stall → first-paint at 2,544 ms). Time-to-render re-measured at this head: median 326 ms vs 383 ms on
merge-base, i.e. −57 ms this round (−48 ms last round); still ~0 under 10 Mbps.
The four documentation gaps are unchanged at this head: the runbook's copy-pasteable delegation command
still throws (re-run just now), build.mjs:200 still says "Set both or neither",
docs/users/features/commands.md:39 still describes one pinned asset, and
docs/verification/export-html-runtime-size/README.md §6 still quotes the renamed log line.
Updated verdict
The thing I flagged as most wanting before merge is now largely covered, and my framing of it was too
strong — I've corrected that above. What this round adds is that the new id test guards a genuinely
exploitable regression on both halves, which I'd call a good use of the round.
Revised pre-merge list, shortest version:
- R1-7 and R1-6 — I'd fold these in now rather than next round. Both are small test edits, and both
have a measured blast radius that is worse than the finding text: one makes every export unopenable,
the other silently drops the renderer branch's only witness. Neither needs a rewrite, just an
element-scoped assertion and aRENDERER_CSS_URLfulfil in the existing route handler. - The delegation runbook command — it is executable and it is broken.
- The CSS budget — decide it explicitly; ~4 lines if the answer is "guard both".
- fix(export): restore document runtime budget headroom #11372 sequencing — still open, still opposite-direction on the same two constants.
R1-18's rAF guard is dead code on every path I could stage; R1-13 has no live impact today. Both are
follow-up material, not merge blockers. R1-8 can be closed as already fixed.
Harness, full-resolution screenshots and raw probe output: https://github.com/wenshao/qwen-code/tree/assets-pr11485
中文版报告（点击展开）
第二轮 —— 在 328feb43f8 上重新验证
装置与上一轮相同：两条基线都从源码构建，浏览器结论来自真实的本地 HTTP 源站（无 CDP 路由拦截）。
merge-base 未变（fbb877a48e），因此第一轮的数字在注明处继续有效。下文凡是写"实测"的，都在本机跑过。
先更正我上一份报告里的一处错误
我写过："如果明天有人删掉 <head> latch，整个测试套件不会有任何反应。" 这是错的。
scripts/tests/export-transcript-document-template.test.js（由 latch 提交本身引入）在模板被还原成
latch 前的形态时会 6 条全红：
latches stylesheet failures in <head>, ahead of the <link> ×
nonces the latch script, because the CSP allows no inline script ×
listens for error in the capture phase ×
only records the failure while the parser is still in <head> ×
acts on the latch from the body script ×
compares the failing element id both listeners agree on × Tests 6 failed (6)
第一轮真正成立的是更窄的那句：浏览器门禁没有见证它 —— 新增的
fails closed when the CDN stylesheet is unavailable 用例在 latch 前的构建上依然通过。所以我当时的第 1 条
诉求说重了：静态那条流水线是实打实的覆盖，缺的只是行为层面的那一半。
新提交确实做到了它声称的事 —— 用变异测试在静态与浏览器两侧都核对过
产物与 d54fcd0f18 逐字节相同（export-transcript-document.js sha256 d87a95d4…、.css e0e4a141…），
因此第一轮的体积、渲染一致性与 fail-closed 矩阵原样成立。这里仍然重跑了一遍：241 条测试全绿
（159 scripts + 76 cli + 6 浏览器门禁），12 格 fail-closed 矩阵在每个故障格都是 closed，
与 merge-base 的渲染差异在两种主题下仍是 0 个像素。
这条 id 契约测试瞄得很准，而且它钉住的两个监听器都是承重的。 我先复现了提交自己的静态结果，
再问了一个静态流水线回答不了的问题：它现在能抓到的这种漂移，真的会坏事吗？
| 变异 | 旧测试文件（d54fcd0f） |
新测试文件（328feb43） |
真实源站下的浏览器行为 |
|---|---|---|---|
| latch 比对了错误的 id | 5 passed |
1 failed | 5 passed |
4.0 MB 导出 + 瞬时 CSS 404 → 无样式渲染（小导出仍 fail-closed） |
| body 监听器比对了错误的 id | 5 passed |
1 failed | 5 passed |
小导出 + 404 / 延迟 404 / SRI 不匹配 → 无样式渲染；4.0 MB 导出 + 延迟 404 → 无样式渲染 |
也就是说，两者覆盖的是互不相交的时间窗——latch 接住"在 body 脚本存在之前派发"的失败，body 监听器接住
"之后派发"的失败——谁都不多余。有一点值得说清楚：这条测试是目前唯一钉住这两者的东西，它对得起自己的位置。
build.mjs 里被更正的注释在事实上是对的。 在导出文档里实测：经文档 createElement 垫片创建的
<style> 会被打上 nonce 并生效（rgb(1,2,3)）；绕过垫片创建的同样的 <style> 会被 CSP 拦截，
报 Applying inline style violates ... 'style-src-elem 'nonce-…''。旧注释所说"CSP 会拦掉未被剥离的重复注入"
是错的，这次更正才是准确的。
设计文档确实同步了，两个语种都是：第 1 节引用了实际发布的 TRANSCRIPT_CSS_ENTRY_FILTER 并点名
transcript-css-entry.mjs；第 2 节描述了 <head> latch（位置、nonce、捕获阶段、只记录、由 body 消费）；
第 3 节点名了模块级渲染守卫；"Files affected" 补齐了此前遗漏的三个文件。
被推迟的发现 —— 在这里替作者跑完，因为浏览器流水线在其主机上跑不动
PR 上的回复把若干发现推到下一轮，理由是它们需要 Playwright/Chromium 加一次构建。这两样我都有，所以我跑了。
下表每一行都是：对 head 树施加一个变异 → 重跑测试 → 在真实浏览器里打开这个变异体。
| 发现 | 结论 | 见证 |
|---|---|---|
| R1-8 发布门禁没有反向测试 | 在 head 上已修复 | 删掉 prepare-package.js 那一行会让 package asset scripts > fails packaging when the published stylesheet is missing 变红（1 failed | 35 passed） |
R1-7 <link> 的 nonce 未按元素定位 |
成立，而且比原文更严重 | 删掉 <link> 的 nonce 后 html.test.ts 仍 3 passed、scripts 流水线 42 passed；而真实浏览器里样式表被 CSP 拦截，每一个导出文件都只会显示加载失败页 |
| R1-6 渲染器门禁被混淆 | 成立 | 把 transcript-renderer 从 body 监听器里删掉后，fails closed when the CDN renderer is unavailable or fails integrity 依然通过；而此时"只有 JS 404、CSS 正常"会得到一个完全空白的页面 —— data-render-complete 未设置、没有 alert、body 文本 0 字符 |
R1-18 两个 renderComplete 守卫，其一无测试 |
已定论 | 模块级守卫才是承重的，而且已被钉住（删掉它 → 新增的样式表用例变红；行为上会把无样式 transcript 盖在报错页上）。rAF 守卫是死代码：删掉它门禁仍 6 passed，文档照样 fail-closed |
| R1-13 层叠顺序无测试 | 成立，但今天没有实际影响 | 把 <link> 放到内联 <style> 之前，所有套件仍全绿（静态 6、html 3、门禁 6/6），渲染逐像素一致（compare -metric AE = 0） |
R1-21 sheetLoaded oracle |
成立，但需要一处更正 | 在带 integrity 的情况下：404 → sheet !== null、0 条规则；连接重置 → sheet !== null；但空响应与截断响应读到的是 sheet === null，因为 SRI 先把它们拦了。真正能骗过这个 oracle 的只有 404 和重置，而不是"空/截断/404" |
| R1-5 发布门禁只校验 JS | 成立 | grep -rn export-transcript-document .github/workflows/ 只有 2 处命中，都是 release-vscode-companion.yml 里的 JS 拉取及其 cmp；全局没有 .css |
| R1-4 该窗口内委派不可用 | 成立 | unpkg：.css 在 0.23.2、0.23.1、0.23.1-preview.0 上都是 404；npm latest = 0.23.2；runbook 里的两开关命令仍会抛错 |
面板 1、2 是两个"测试全绿也能发出去"的变异。面板 3 是新增样式表用例确实抓到的那个 —— 注意它同时满足
data-render-complete === 'error' 却仍然显示了 transcript，因为 React 在 showLoadError 写入之后又替换了
#app；也就是说，只读这个标记的检查会通过，而读者看到的是一个无样式的导出。
与第一轮相同的部分
体积（4,139,386 → JS 1,833,941 + CSS 2,302,905；总量 −2,540 字节；gzip −2,110）、字节棘轮反证
（CSS +1 MB：本 PR 全绿，merge-base 报 Document export runtime is 5142389 bytes; expected <= 4200000
—— 在本轮 head 上重跑，仍然全绿），以及渲染阻塞实测（样式表拖延 2.5 s → first-paint 落到 2,544 ms）。
渲染耗时在本轮 head 上重测：中位数 326 ms，对 merge-base 的 383 ms，即本轮 −57 ms（上轮 −48 ms）；
10 Mbps 限速下仍然约等于 0。
四处文档缺口在本轮 head 上没有变化：runbook 里可复制的委派命令仍会抛错（刚刚重跑确认）、
build.mjs:200 仍写着 "Set both or neither"、docs/users/features/commands.md:39 仍描述只有一个版本固定资产、
docs/verification/export-html-runtime-size/README.md §6 仍引用被改名掉的日志行。
更新后的结论
我此前标为"最希望在合入前解决"的那一项，现在基本已被覆盖，而且我当时的说法过重 —— 上文已更正。
本轮新增的信息是：新的 id 测试守住的是一个在两侧都真实可利用的回归，我认为这一轮用得很值。
修订后的合入前清单（最短版）：
- R1-7 与 R1-6 —— 我建议现在就并进来，而不是留到下一轮。两者都只是很小的测试改动，而实测的影响面
都比发现原文更严重：一个让所有导出都打不开，另一个悄悄弄丢了渲染器分支唯一的见证。都不需要重写，
只要一处按元素定位的断言，以及在现有 route handler 里补上RENDERER_CSS_URL的 fulfil。 - 委派 runbook 里的命令 —— 它是可执行的，而且是坏的。
- CSS 预算 —— 显式做一次决定；如果答案是"两个都守"，大约 4 行。
- fix(export): restore document runtime budget headroom #11372 的先后顺序 —— 仍然开着，仍然在同样那两个常量上反方向改。
R1-18 里的 rAF 守卫在我能构造的所有路径上都是死代码；R1-13 今天没有实际影响。这两条属于后续跟进，
不是合入阻断项。R1-8 可以按"已修复"关闭。
Harness、原分辨率截图与原始探针输出：https://github.com/wenshao/qwen-code/tree/assets-pr11485
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 51 passed · 2 failed · 53 total Flakiness gate: 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：51 通过 · 2 失败 · 53 总计 抖动门： Verification reportPR #11485 deep verification —
|
| Metric | base 33a40625 |
head 328feb43 |
Δ |
|---|---|---|---|
export-transcript-document.js (shipped bytes) |
4,139,823 | 1,833,941 | −2,305,882 |
export-transcript-document.css (shipped bytes) |
— (not produced) | 2,302,905 | +2,302,905 |
| Total CDN download (js + css) | 4,139,823 | 4,136,846 | −2,977 |
| gzip −9 total transfer | 1,538,785 | 1,536,599 (527,026 + 1,009,573) | −2,186 |
| build's own logged number | Document export runtime is 4142828 bytes |
renderer JS is 1833944 bytes; CSS is 2302905 bytes |
— |
document.html template |
6,606 | 7,917 | +1,311 |
| byte-budget cap / headroom | 4,200,000 / 57,172 | 1,930,000 / 96,059 | ratcheted down |
| component CSS inside the JS | present | absent | — |
| CSS runtime-injection line inside the JS | present | absent | no double delivery |
Every residual byte is accounted for. The JS shrank by 2,305,882 while the CSS asset is 2,302,905, so
total download fell by exactly 2,977. That decomposes as: 2,699 bytes of JSON string-escaping overhead
no longer paid (measured independently — the literal line in transcript.js is 2,305,604 bytes, the raw
CSS after JSON.parse is 2,302,905) plus 278 bytes net of the removed runtime-injection line against
the two new document-main.tsx guards. No unexplained residue.
The extraction is exact. An independent script that re-implements the lift
(source.match(/^const __qwenWebShellCss=("(?:[^"\\]|\\.)*");\n/) → JSON.parse) produces CSS whose
sha256 is e0e4a14164081338… — identical to the build's export-transcript-document.css, and identical
to the copy in dist/ that npm publishes and unpkg serves. The ^-anchored regex without the m flag does
match at index 0 of the real packages/web-shell/dist/transcript.js, and the injection line it steps over is
exactly 367 bytes, the number the build.mjs comment claims.
Corrections to the PR description (facts, not change requests):
- The description's Before/After table gives 1,831,301 JS and 2,302,457 CSS. At this merge commit the
reproducible numbers are 1,833,944 logged / 1,833,941 shipped JS and 2,302,905 CSS
(Δ +2,640 / +448), because the author built against their ownweb-shelloutput. The table's Before
figure of 4,136,297 bytes at 0.23.2 is exactly right — I downloaded the published asset from unpkg and
it is 4,136,297 bytes. - The table row "
__qwenWebShellCssliteral in JS: present → absent" describes the pre-bundle input, not
the shipped asset. esbuild's minifier mangles that identifier away in both arms:grep -c __qwenWebShellCssis 0 on the base bundle too. The verifiable claim is the CSS content moving out,
which does hold (a distinctive rule,.katex{font: 1.21em KaTeX_Main,Times New Roman,serif, is present in
the base bundle and absent from the head bundle). - "The build's byte budget was within ~60 KB of its hard cap" is confirmed: base logged 4,142,828 against a
4,200,000 cap = 57,172 bytes of headroom, and the base build did emit its own over-warning line.
A hazard I expected and disproved. Moving CSS from an inline <style> to an external <link> changes the
base URL every relative url() resolves against — from the exported document to https://unpkg.com/…. That
would silently break fonts and images. It does not apply here: the lifted stylesheet contains 60 url()
references and all 60 are data: URIs, zero relative (including all 20 @font-face blocks, so the KaTeX
woff2 fonts travel inline). font-src data: in the CSP admits them. This is the sharpest consequence of the
design and it is inert.
Findings
F1 (Suggestion) — the repo's own documented renderer-delegation recipe now exits 1, and delegation is impossible against every published version
build.mjs gained a hard guard: QWEN_EXPORT_RENDERER_CSS_INTEGRITY must be set together with
QWEN_EXPORT_RENDERER_IDENTITY. The guard's reasoning is sound — a delegated renderer points the JS and CSS at
the same published version, so the CSS digest must describe that published asset. But docs/verification/export-renderer-delegation-mermaid/README.md:105-109
is a runnable recipe that sets only the two original variables, and the PR does not update it.
Reproduce (a scratch worktree at the merge commit, so the head dist/ is untouched):
git worktree add tmp/deleg-tree HEAD && cd tmp/deleg-tree
QWEN_EXPORT_RENDERER_IDENTITY='0.23.1-preview.0+d7962879afdccd34' \
QWEN_EXPORT_RENDERER_INTEGRITY='sha384-CVacTzaM6pEzmp3UrBJQ/WMSVZfvRxbrNJtCf1c03j4Gox5y9dqndkBoTQ3ktzzh' \
node packages/web-templates/src/export-html/build.mjs
# Error: QWEN_EXPORT_RENDERER_CSS_INTEGRITY must be set together with the renderer delegation: …
# exit 1The recipe cannot simply be amended with a third hash, because no published version has the CSS yet.
Probed against live unpkg:
| version | .js |
.css |
|---|---|---|
0.23.2 (current package.json, released 2026-09-09) |
200 | 404 |
0.23.1-preview.0 (the version the doc recipe delegates to) |
200 | 404 |
0.23.1 |
200 | 404 |
0.23.0 |
404 | 404 |
So until the next release publishes the asset, the delegation knob is dead for every version one could
delegate to, and the doc that describes it is wrong in two ways (the recipe throws; its closing sentence
"build.mjs throws if exactly one of the two is set" now describes a three-variable contract).
Blast radius, bounded — this is not a shipping regression for users:
- Users on published 0.23.2 get the pre-split template with no
<link>at all. Unaffected. - Users on the next published release get both assets published at that same version. Unaffected —
prepare-package.jsrequires the CSS indist/and lists it infiles[], so the two cannot diverge. - Source/dev builds at 0.23.2 already fail closed before this PR, for an independent reason. I hashed the
published asset and both builds: published@0.23.2JS issha384-fw8bWYGocO+WEj1+/97lmsi1VF8i5Yl9aur4TezgkOGrCUi+9D8URaZP3/ADoEq6,
while the base build issha384-GMR6aZ1phvGqcmXh0FQ4VqynBnKkJv9WnoeyTqIM/1DuAuNgmDysWCIEUsw2LTJzand head is
sha384-ifOL6KUFEseEhIv7BsjEnk4Z39UdyEDDCwiiOaj0iezrZQMYgeopnhL8/Xhti/dH. Neither matches, so a document built
frommainat 0.23.2 was already rejected by the renderer's own SRI check. The CSS 404 adds a second
reason to reach the same page; it does not convert a working export into a broken one. - No CI workflow sets
QWEN_EXPORT_RENDERER_*(grep -rlover.github/returns nothing), so no automation
breaks. The knob is manual-only.
Suggested minimal fix (docs only — not applied, not measured against a suite)
Update docs/verification/export-renderer-delegation-mermaid/README.md to show all three variables and to
state that delegation requires a published version that carries both assets, i.e. the first release after
this split. No source change is implied: the guard is correct, and the design docs already say the knob "gains
a parallel QWEN_EXPORT_RENDERER_CSS_INTEGRITY".
Related, and lower still: .github/workflows/release-vscode-companion.yml:103-111 ("Verify published export
renderer") downloads the published JS at the current version and cmps it against the local build — a
release-time check that what unpkg serves is what was built. The CSS now has no equivalent cmp, so the
release lane verifies one of the two assets the document depends on. prepare-package.js makes a missing CSS
fatal at packaging time, which is why I rate this a nit rather than a gap with teeth.
F2 (Suggestion) — the <head> stylesheet latch is not behaviourally load-bearing at any document size I could construct
The stylesheet failure is defended by three layers: (a) the <head> latch that records
window.__transcriptStyleFailed, (b) the body listener's new transcript-stylesheet id branch, and
(c) the two document-main.tsx guards that refuse to render over an error body. Layered guards hide each
other, so I reverted each alone and the pair together, and judged every row with the PR's own browser gate case.
The first run of this matrix was void and I am reporting the corrected one. Driver v1 rebuilt with
node build.mjs, which regenerates src/generated/*.ts but not the compiled
packages/web-templates/dist/ that the integration test imports through @qwen-code/web-templates
(package main = dist/index.js, frozen at 00:10:14 while my mutants ran at 00:34). All five browser rows
therefore executed the unmutated template and reported a uniform, meaningless "survived". Driver v2 runs
the full npm run build -w @qwen-code/web-templates (node build.mjs && tsc) and probes the consumed
artifact before each browser run, so a mutation that fails to reach it is reported INVALID instead of being
allowed to masquerade as a survivor. Witness: 02-mutation-matrix-fail-closed-layers.png.
| row | reachability probe (compiled template / bundle) | browser gate | template unit test |
|---|---|---|---|
| control (unmutated) | latch=1, id-cmp=2, main-guards=2 ✔ | green (1 passed) | green (6 passed) |
a — revert <head> latch only |
latch=0, id-cmp=1, main-guards=2 ✔ | green — SURVIVED | red (5 failed / 1 passed) |
| b — revert body-listener id only | latch=1, id-cmp=1, main-guards=2 ✔ | red — killed | red (1 failed / 5 passed) |
| a+b — revert both listeners (combination row) | latch=0, id-cmp=0, main-guards=2 ✔ | red — killed | red (5 failed / 1 passed) |
c — revert both document-main.tsx guards |
latch=1, id-cmp=2, main-guards=0 ✔ | red — killed | green (out of that file's scope) |
The positive control is green on both oracles, so the kills are attributable to the mutations and not to a dead
harness. Provenance of the two columns: the browser column is from driver v2 (logs/mutation-matrix-v2.log,
raw per-row logs in mutants2/); the template-test column is from driver v1 (mutants/*.template.log) and
remains valid, because export-transcript-document-template.test.js reads
src/export-html/src/document-index.html directly rather than the compiled dist/ — the staleness that
voided v1's browser rows cannot reach it. v1's browser column is discarded entirely and is not reported.
Read the shape: (b) and (c) are load-bearing — each alone breaks the gate. (a) is not: removing
the latch alone leaves the gate green, and only the combination row (a+b) shows the two listeners are
jointly load-bearing. Under the skill's classification, (a) is redundant defence with respect to every
behavioural gate in the repo, and is pinned only by the six static assertions in
scripts/tests/export-transcript-document-template.test.js (which do kill it — 5 of 6 go red).
I then went after the race the latch was written for, because "redundant on a 1-record document" is not
"redundant". The code comment and the template test's own comment both state the mechanism as fact — "on a
large document a CSS failure that settles first is dispatched while no listener exists yet: nothing marks the
render as failed … the transcript renders completely unstyled while stamping data-render-complete="true"".
So I re-ran the same case with createMaximumDocument() — the largest envelope the format allows
(EXPORT_TRANSCRIPT_LIMITS_V1.maxBlocks blocks), which is what puts the parser deepest into the body when the
abort settles:
| document | build | result |
|---|---|---|
| maximum-size | control (latch present) | fails closed, green (1,785 ms) |
| maximum-size | latch removed (probe confirmed latch=0 in the compiled template) |
still fails closed, green (1,336 ms) |
I could not reproduce the race at either document size. With the latch gone, the body listener caught the
aborted stylesheet every time. Two honest limits on that negative: the failure is injected as a Playwright
route.abort('blockedbyclient'), whose settling time is shaped by the routing layer rather than by a real 404
or DNS failure, and this is headless Chromium 149 on a loaded shared runner. A real network failure on a
different build could still settle before the body listener registers — the latch may well be correct
defence-in-depth. What is measurable is that no test in the repo demonstrates the failure it prevents, and
that the comments assert a reproduction that this round did not observe.
This is a Suggestion, not a blocker: the guard is additive, fail-safe, cannot make things worse, and the
static test pins its position, nonce, capture phase and id agreement — the ways it could actually be wrong.
The actionable part is the prose. If the author has a reproduction (a real 404 rather than a routed abort, or a
slower device), recording it as a comment or a case would turn six static assertions into a demonstrated
mechanism; if not, the comments in document-index.html and export-transcript-document-template.test.js
overstate what is proven.
Vacuity and gate liveness
- The browser gate is live and reaches the code under test. Proven twice over: the unmutated control is
green, and three of four mutants turn it red with the intended behavioural mismatch (data-render-complete
and therole="alert"text), not with an import or fixture error. The reachability probe on the consumed
artifact is what makes that claim safe — see F2 for the run where it was not. - The template unit test is live: mutant (a) turns 5 of its 6 cases red with
AssertionError: expected -1 to be greater than -1— the position assertion failing because the latch string
is gone, which is the mismatch the test exists to catch. - The packaging tests are live:
scripts/tests/package-assets.test.jscovers the new all-or-nothing copy
branch, the "name the missing stylesheet" warning, andpreparePackageexiting 1 when the published CSS is
absent. All green (45 tests across the three collectible files).
Not covered
scripts/tests/install-script.test.jscould not run in this container, and the failure is environmental —
proven by A/A, not assumed. It throws at import time from its own guard:`zip`/`unzip` missing on a CI host(CI=true,command -v zipempty, only/usr/bin/unzippresent). The identical file at base
33a40625fails the same way (Test Files 1 failed (1),Tests no tests), and the guard is untouched by the
PR (git diff HEAD^1..HEADmatcheszipAvailable0 times). Consequence: the PR's new assertion that
standalone archives excludelib/export-transcript-document.csswas not executed here. I verified the
same intent statically instead — the CSS is inDIST_NPM_PACKAGE_ONLY_ENTRIES— which is weaker.- Per-commit attribution is out of reach. Depth-2 checkout:
git rev-list HEAD^1..HEAD^2returns 1
commit while$QWEN_VERIFY_CONTEXTrecords 5, andgit rev-parse --is-shallow-repositoryistrue. The
1is the plausible-number-at-a-shallow-boundary artefact, not the truth. I verified the aggregate
HEAD^1..HEADdiff only; the four intermediate commits (Windows path matching, the latch, the scripts
naming, the id contract) were exercised as one change, not individually. - Repo-wide
typecheck,lintandprettierwere not run — the PR's own CI covers them and my A/B did
not need the numbers. The changed.mjs/.jstest surfaces were executed by the gates above. - The fail-closed path was only exercised against a routed abort, never a real network 404. I confirmed
unpkg really does return 404 for@​0.23.2/export-transcript-document.css, but I did not drive a browser at
that live URL, because doing so needs a document whose renderer SRI also matches the published asset — which
no source build produces (see F1). So this round reproduces the wire shape of a stylesheet failure, not a
genuine CDN 404 against a real published document. release-vscode-companion.ymlsequencing was reasoned about, not run. I read the step and probed unpkg;
I did not execute a release.- The
export-html-runtime-sizeREADME's reference-point table was not refreshed by the PR, but it is
explicitly labelled as pre-refactor!: retire @qwen-code/webui #9812 history, so I did not treat that as drift. - Windows path matching was verified by unit test only.
TRANSCRIPT_CSS_ENTRY_FILTERaccepts both
separators and rejectsweb-shell/dist/index.js, and I confirmed the POSIX branch fires for real (the
extraction happened). I have no Windows host, so the[\\/]class is unexercised end-to-end.
Methodology
Environment: the CI verify container (node:22-bookworm, Node v22.23.2, npm 10.9.8, CI=true, uid 1000),
working tree at the merge commit 9358e45d with npm ci and npm run build already completed. Network was
available and used read-only against unpkg.com for the published-asset probes; nothing was posted anywhere.
The A/B used a scratch git worktree at HEAD^1 under tmp/, deliberately sharing the head tree's
node_modules and packages/web-shell/dist so that both arms bundle byte-identical input — the realpath of
every internal dependency was asserted from inside the base worktree before either build ran, and both
worktrees were removed afterwards (git worktree list shows only the main tree; git status is clean and the
final artifact sha256s match the pristine head build). A second scratch worktree hosted the delegation-recipe
reproduction so a throwing build's rm -rf of dist/ could not disturb the head tree. Oracles were: file
existence and exact byte counts, sha256/sha384 digests compared against the values embedded in the built
document.html, substring presence of a distinctive CSS rule and of the injection marker in each bundle, and
the real headless Chromium gate — I installed Playwright's chromium into the container (npx playwright install chromium, revision 1228, launches as Chromium 149.0.7827.55) so the PR's own browser assertions ran for real
rather than being taken on trust. Harnesses live in the artifact directory as .mjs/.sh and are rerunnable:
ab-harness.mjs (30 assertions), mutation-matrix-v2.sh + adjudicate-matrix.mjs (the corrected matrix),
mutation-matrix.sh (the void v1, kept because the reason it is void is itself the finding). Raw per-cell
output is under logs/ — ab-harness.log, integration-head.log (6/6), unit-cli.log (76 passed),
unit-scripts.log (45 passed), mutation-matrix-v2.log, and published-0.23.2-renderer.js, the 4,136,297-byte
asset downloaded from unpkg that anchors the description's Before figure.
Flakiness gate log
integration test, out of gate scope: integration-tests/chat-transcript-document.test.ts
rounds=5 files=5 skipped=1
file packages/cli/src/ui/utils/export/formatters/html.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/utils/export/formatters/html.test.ts
file scripts/tests/export-transcript-document-template.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/export-transcript-document-template.test.js
file scripts/tests/install-script.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/install-script.test.js
file scripts/tests/package-assets.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/package-assets.test.js
file scripts/tests/transcript-css-entry-filter.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/transcript-css-entry-filter.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/ui/utils/export/formatters/html.test.ts: PPPPP
scripts/tests/export-transcript-document-template.test.js: PPPPP
scripts/tests/install-script.test.js: FFFFF
scripts/tests/package-assets.test.js: PPPPP
scripts/tests/transcript-css-entry-filter.test.js: PPPPP
verdict: consistent-fail
summary: 1 of 5 changed test file(s) failed identically in every round — deterministic, so CI owns that signal
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 1 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 1 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 1 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 00:11:27
�[2m Duration �[22m 413ms�[2m (transform 112ms, setup 21ms, collect 0ms, tests 0ms, environment 0ms, prepare 61ms)�[22m
round 1 · scripts/tests/package-assets.test.js: P (exit 0)
round 1 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
round 2 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 2 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 2 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 2 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 00:11:37
�[2m Duration �[22m 456ms�[2m (transform 117ms, setup 24ms, collect 0ms, tests 0ms, environment 0ms, prepare 77ms)�[22m
round 2 · scripts/tests/package-assets.test.js: P (exit 0)
round 2 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
round 3 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 3 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 3 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 3 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 00:11:47
�[2m Duration �[22m 409ms�[2m (transform 113ms, setup 18ms, collect 0ms, tests 0ms, environment 0ms, prepare 72ms)�[22m
round 3 · scripts/tests/package-assets.test.js: P (exit 0)
round 3 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
round 4 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 4 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 4 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 4 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 00:11:57
�[2m Duration �[22m 430ms�[2m (transform 115ms, setup 19ms, collect 0ms, tests 0ms, environment 0ms, prepare 63ms)�[22m
round 4 · scripts/tests/package-assets.test.js: P (exit 0)
round 4 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
round 5 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 5 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 5 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 5 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 00:12:07
�[2m Duration �[22m 426ms�[2m (transform 116ms, setup 24ms, collect 0ms, tests 0ms, environment 0ms, prepare 81ms)�[22m
round 5 · scripts/tests/package-assets.test.js: P (exit 0)
round 5 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
Closed out the remaining review batch in
Not taking three non-blocking suggestions in this PR: a cascade-order regression assertion (there are no overlapping selectors today), isolating the renderer-failure test route, and extending the separate release workflow published-asset comparison to CSS. This PR has already been through multiple review rounds; these are follow-ups rather than another scope expansion. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Follow-up review and simplification pass:
Verification:
All 15 inline review threads are resolved. I did not add a CSS/link ordering pin or duplicate CSS verification in the release workflow: prior evidence found no selector overlap or rendering difference from the order, and package preparation already requires the CSS artifact. Those remain non-blocking suggestions and are intentionally deferred to avoid widening this already mature review. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] Blocking finding(s) follow.
Partially reviewed — gaps disclosed.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-3 byte ratchet no longer covers the lifted CSS asset (packages/web-templates/src/export-html/build.mjs:354) — already reported (comment 5603265110, stage-2 item 1; also comments 5603265631 and 5603264729; re-measured by @wenshao in comme…
- R1-20 delegation docblock and runbook still describe a two-variable contract (packages/web-templates/src/export-html/build.mjs:234) — already reported (comment 5603265110, stage-2 item 3; also comment 5603265631; executed by @wenshao in com…
- R1-5 release-time published-asset gate compares only the JS (packages/web-templates/src/export-html/build.mjs:392) — already reported (comment 3971657972)
- R1-23 stylesheet failures route into the renderer-only diagnostic (packages/web-templates/src/export-html/src/document-index.html:77) — already reported (comment 3971657977)
- The head latch has no behavioural witness (integration-tests/chat-transcript-document.test.ts:718) — already reported by @wenshao (comment 5609994613 §3 item 1 and its verdict item 1)
Unresolved, please confirm:
- [Critical] stage-3 triage blocker (comment 5603265631) — whether open PR #11372 or this PR lands first. #11372 is verified still OPEN at this head and still edits only packages/web-templates/src/export-html/build.mjs (+6/-8), moving the same two budge…
Not reviewed: build-and-test on Windows and macOS — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) are merge_group/schedule-gated and report skipped at this commit, and no suite ran on either host here; this is the lane class that would have executed the two new scripts/tests files on a non-POSIX filesystem, which is what round-1 Critical R1-1 turned on. Both were reasoned safe (neither new file shells out, neither is in the win32 exclude list, .gitattributes sets * text=auto eol=lf) but neither was observed running there..
Not reviewed: test-efficacy probe — the positive control never ran (no probe file was green in the unmutated baseline), so hunk-level test gating is unmeasured for this PR; harnessValidated is null, neither validated nor refuted, and 20 of 20 hunk probes were skipped (14 over the cap, 6 for baseline).
Not explored to full depth (tool budget reached): "agent 6b": integration-tests/chat-transcript-document.test.ts was not executed here (it needs the full browser gate and a 60 s maximum-document case); I substituted a sa…; "agent 6b": Firefox and WebKit nonce-on-external-stylesheet behaviour is unverified — Firefox is installed but cannot launch on this host (missing system dependencies), and…; "agent 1a": did not execute integration-tests/chat-transcript-document.test.ts (Playwright + Chromium and a full npm run build && npm run bundle prerequisite), so the n…; "agent 1a": did not execute scripts/tests/install-script.test.js (shells out to create-standalone-package.js with a fake Windows node archive), so the new lib/export-t…; "agent 1a": did not re-run node src/export-html/build.mjs from scratch — I verified the already-present dist/ artifacts instead, so the byte-budget conclusion rests on ….
Test Plan (not a blocker): 6 passed — this review observed 29641 passed.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
docs/design/2026-09-09-split-export-transcript-css.md:30 — [probe] D3-1 design docs carry the budget-squeeze justification the issue triage retracteddocs/design/2026-09-09-split-export-transcript-css.md:57 — [probe] D3-2 issue condition F's CSS byte breakdown never performed or recordeddocs/verification/export-html-runtime-size/README.md:114 — [probe] D3-6 §3 reconciliation rule compares whole-runtime rows against the JS alonepackages/web-templates/src/export-html/build.mjs:50 — [probe] D3-3 re-ratcheted budget constants record no measured baselinepackages/web-templates/src/export-html/build.mjs:51 — [probe] D3-7 re-ratchet falsifies the sibling runbook's live §2packages/web-templates/src/export-html/build.mjs:353 — [probe] D3-5 size expression also dropped the inline document CSS operandpackages/web-templates/src/export-html/build.mjs:426 — [probe] D3-4 residual-placeholder guard enumerates names instead of the class
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：build-and-test on Windows and macOS — Test (windows-latest, Node 22.x) and Test (macos-latest, Node 22.x) are merge_group/schedule-gated and report skipped at this commit, and no suite ran on either host here; this is the lane class that would have executed the two new scripts/tests files on a non-POSIX filesystem, which is what round-1 Critical R1-1 turned on. Both were reasoned safe (neither new file shells out, neither is in the win32 exclude list, .gitattributes sets * text=auto eol=lf) but neither was observed running there..
未审查（原文为英文）：test-efficacy probe — the positive control never ran (no probe file was green in the unmutated baseline), so hunk-level test gating is unmeasured for this PR; harnessValidated is null, neither validated nor refuted, and 20 of 20 hunk probes were skipped (14 over the cap, 6 for baseline).
未探索到全部深度（达到工具调用预算）："agent 6b"：integration-tests/chat-transcript-document.test.ts was not executed here (it needs the full browser gate and a 60 s maximum-document case); I substituted a sa…；"agent 6b"：Firefox and WebKit nonce-on-external-stylesheet behaviour is unverified — Firefox is installed but cannot launch on this host (missing system dependencies), and…；"agent 1a"：did not execute integration-tests/chat-transcript-document.test.ts (Playwright + Chromium and a full npm run build && npm run bundle prerequisite), so the n…；"agent 1a"：did not execute scripts/tests/install-script.test.js (shells out to create-standalone-package.js with a fake Windows node archive), so the new lib/export-t…；"agent 1a"：did not re-run node src/export-html/build.mjs from scratch — I verified the already-present dist/ artifacts instead, so the byte-budget conclusion rests on …。
Test Plan（非阻断）：6 passed — this review observed 29641 passed。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 7 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.2)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 292 passed · 0 failed · 292 total Flakiness gate: 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：292 通过 · 0 失败 · 292 总计 抖动门： Verification reportPR #11485 deep verification (round 2) —
|
| # | Round-1 finding | Sev | Status at 30d8989a |
|---|---|---|---|
| F1 | Renderer-delegation doc recipe exits 1; delegation impossible against every published version | Suggestion | fixed — the doc now shows all three variables and says the recorded 0.23.1-preview.0 target predates the split. Re-measured: all 6 partial combinations throw from their intended guard, and the three-variable recipe works end-to-end (03-delegation-three-variable-contract.png). The "no published version has the CSS" half is still true (6 versions probed, all 404) but the doc now states it, so it is release sequencing, not a defect. |
| F2 | The <head> stylesheet latch is not behaviourally load-bearing at any document size constructible |
Suggestion | stands — strengthened. Round 1 could only report a failed reproduction under a routed abort. This round reproduces the failure on a real HTTP 404 and a real SRI mismatch, and row b of the matrix proves positively that the latch's flag is set yet the document still renders and stamps renderComplete="true". The comment's mechanism claim is falsified by mutation, not merely unobserved. |
| — | Round-1 nit: release-vscode-companion.yml cmps the published JS but has no equivalent for the CSS |
Nit | stands (unchanged by the delta; the delta does not touch .github/). Still mitigated by prepare-package.js making a missing CSS fatal at packaging time. |
| — | Round-1 "Not covered": fail-closed never exercised against a real network 404 | — | closed. All CSS-failure rungs this round are real wire failures served by a loopback HTTP server, with no Playwright routing installed at all. |
| — | Round-1 "Not covered": install-script.test.js uncollectable (no zip) |
— | stands, still environmental. A/A re-proven at the new base; apt-get install zip is refused (not root). |
Round 1 also reported a deferred/declined item worth re-measuring: the escaping-artifact class of concern
does not apply here, and no accepted-tradeoff list in the description changed its numbers — but one row of it
did move, see Corrections below (link.sheet as an SRI oracle).
Scope selection
Central claim. The web-shell component stylesheet is lifted out of export-transcript-document.js into a
separate version-pinned, SRI-protected export-transcript-document.css, so the JS a browser must
download/parse/compile before rendering drops from ~4.14 MB to ~1.84 MB, with the CSS fetched in parallel.
Secondary claim 1. A stylesheet load failure fails closed with the same load-error page as a missing
renderer. Delta focus: the closeout commit removed one of the two document-main.tsx guards that
round 1's matrix showed were jointly load-bearing, so this claim needed re-proving, not re-reading.
Secondary claim 2. The delegation knob's new three-variable contract behaves as the rewritten doc says.
Delta since round 1 (328feb43..30d8989a, 2 commits, 12 files, +37/−92): the doc rewrite (F1), removal
of the renderComplete !== 'error' guard and its design-doc bullets in both languages, the alert text
("published renderer or stylesheet"), the gate now fulfilling the CSS route inside the renderer-failure
loop, new html.test.ts link assertions, and three large comment deletions. New probes were scoped to
exactly that: the guard-removal matrix (rows c/d), the real-wire fail-closed ladder, the delegation
contract, and vacuity of the new html.test.ts assertions.
Budget: A/B pair with the esbuild-parity redo (~20 min), real-HTTP fail-closed ladder (~10 min), 6-row
mutation matrix with reachability probes (~12 min), delegation contract (~8 min), vacuity mutants (~8 min),
gates (~5 min).
Central claim + A/B
Both arms are real node src/export-html/build.mjs runs in scratch worktrees under tmp/. The bundler
input is byte-identical across arms and that is asserted, not assumed: git diff HEAD^1..HEAD -- packages/web-shell is 0 files; sha256(packages/web-shell/dist/transcript.js) =
afc58db3728276483988… (3,537,330 bytes) is the single shared input; neither worktree has a node_modules
of its own, and readlink -f node_modules/@​qwen-code/web-shell = /__w/qwen-code/qwen-code/packages/web-shell
for both. Witness: 01-ab-bytes-sri-base-vs-head.png.
A confound this round caught and removed. package-lock.json pins
packages/web-templates/node_modules/esbuild at 0.21.5 (vite 5's copy), so the production build resolves
esbuild there. A fresh worktree lacks that nested directory and silently walks up to the root's 0.25.6,
whose minifier emits different bytes (t.flags&4098&& vs (t.flags&4098)!==0&& — 437 bytes and ~61,810
differing character positions across the bundle). My first A/B ran both arms on 0.25.6: internally
consistent, but not comparable to CI or to the author's numbers (1,836,353 vs 1,835,916 logged). I symlinked
the production nested node_modules into both worktrees, rebuilt both arms, and added two scripted
bundler-parity assertions. The table below is the esbuild-0.21.5 (production-faithful) run.
| Metric | base 005fc97b |
head 30d8989a |
Δ |
|---|---|---|---|
| esbuild resolving the build | 0.21.5 | 0.21.5 | asserted equal |
export-transcript-document.js (shipped bytes) |
4,141,408 | 1,835,913 | −2,305,495 |
export-transcript-document.css (shipped bytes) |
— (not produced) | 2,302,905 | +2,302,905 |
| Total CDN download (js + css) | 4,141,408 | 4,138,818 | −2,590 |
| gzip −9 total transfer | 1,526,420 | 1,525,041 (529,184 + 995,857) | −1,379 |
| build's own logged number | Document export runtime is 4144413 bytes |
renderer JS is 1835916 bytes; CSS is 2302905 bytes |
— |
document.html template |
6,606 | 7,931 | +1,325 |
| byte-budget cap / headroom | 4,200,000 / 55,150 | 1,930,000 / 94,084 | cap ratcheted down |
| component CSS content inside the JS | present | absent | — |
| CSS runtime-injection line inside the JS | 1 occurrence | 0 | no double delivery |
Every residual byte is accounted for. JS shrank by 2,305,495 while the CSS asset is 2,302,905, so total
download fell by 2,590. Decomposition: 2,698 bytes of JSON string-escaping no longer paid (the literal
line in transcript.js is 2,305,604 bytes; the raw CSS after JSON.parse is 2,302,905) plus −108 net of
the removed 367-byte runtime-injection line against the two new document-main.tsx guards and the longer
alert string. No unexplained residue.
The extraction is exact. An independent re-implementation of the lift
(source.match(/^const __qwenWebShellCss=("(?:[^"\\]|\\.)*");\n/) → JSON.parse) matches at index 0 of the
real packages/web-shell/dist/transcript.js and produces CSS whose sha256 is e0e4a14164081338… —
byte-identical to the build's asset. The injection line the plugin steps over is 367 bytes including its
newline (366 + \n), which is what build.mjs actually removes, so the comment's number is right.
A hazard expected and disproved again. Moving CSS from an inline <style> to an external <link>
changes the base URL every relative url() resolves against. Still inert: the lifted sheet has 60 url()
references, all 60 data:, zero relative, including all 20 @font-face blocks, and the CSP's
font-src data: admits them.
Cascade actually happens, measured two independent ways (control rungs of the fail-closed ladder):
2,385 rules in the loaded sheet, 17 of the first 4,000 selectors match live elements, and disabling the
<link> moves computed styles on rendered elements (fontFamily/color/backgroundColor/padding/
borderRadius/display over 400 elements). Two instruments with independent failure modes agree.
Corrections
link.sheet !== nullis not an SRI oracle, and the delta already walked the claim back — measurement
confirms the walk-back was correct. Round 1 quoted the gate's comment that a non-nullsheet"proves the
SRI check passed and the CSS parsed". Measured this round on real wire failures: a genuine HTTP 404
leaveslink.sheetnon-null (an empty sheet —sheet=true,cascades=false) at every delay rung, while
a genuine SRI mismatch nulls it (sheet=false). So that assertion cannot detect a missing CSS asset at
all. The closeout commit replaced the comment with "Keep a smoke-check for the stylesheet link, while the
KaTeX font-family is the cascade oracle" — which matches the measurement exactly. This is a correction to
the earlier description of the code, not a request to change it; the load-bearing oracle in the gate is
the KaTeXfont-family, and it is asserted.- The description's Before/After numbers are the author's local build, not this merge commit. The table
gives 1,831,301 JS / 2,302,457 CSS; reproducible here at 1,835,916 logged / 1,835,913 shipped JS and
2,302,905 CSS (Δ +4,615 / +448), because the author built against their ownweb-shelloutput. The
Before figure is exactly right: I downloaded the published asset and@0.23.2/export-transcript-document.js
is 4,136,297 bytes. - The "
__qwenWebShellCssliteral: present → absent" row describes the pre-bundle input, not the shipped
asset — esbuild's minifier mangles that identifier away in both arms (grep -cis 0 on base too). The
verifiable claim is the CSS content moving out, which holds, and the injection line disappearing
(1 → 0 occurrences ofdocument.querySelector).
Findings
F2′ (Suggestion, carried forward and strengthened) — the <head> latch's comment states a mechanism this round falsifies by mutation
Three layers defend a stylesheet failure: (a) the <head> latch recording
window.__transcriptStyleFailed, (b) the body listener's transcript-stylesheet id branch, and (c)
the module-scope mount guard in document-main.tsx. Layered guards hide each other, so I reverted each alone
and in combination, and — because round 1's first matrix was voided by a stale compiled artifact — every row
probes both consumed artifacts before its result is believed: the compiled template
(packages/web-templates/dist/generated/exportTranscriptDocumentTemplate.js, which the CLI formatter imports
at runtime) and the renderer bundle
(packages/web-templates/src/export-html/dist/export-transcript-document.js, which the gate reads directly).
A row whose probe misses its expectation is reported INVALID; all six rows report REACHED.
Witness: 02-mutation-matrix-fail-closed-layers.png.
Oracles per row: the repo's own browser gate (stylesheet case, routed abort), a real-HTTP harness (9 rungs,
81 assertions — loopback server, no Playwright routing, genuine 404 / SRI mismatch / empty body, server-side
delays 0/800/2500 ms, plus the maximum-size document at maxBlocks=1000), and the static template test.
| row | reachability probe (template latch / sheetId, bundle renderComplete) | repo gate | real-HTTP harness | template test |
|---|---|---|---|---|
| control | 2 / 3 / 3 ✔ | green (2 passed) | 81 / 0 / 81 | green (6) |
a — revert <head> latch (head script + body reader) |
0 / 2 / 3 ✔ | green — SURVIVED | 75/6 — the 6 are only the latch-mechanism assertion; every fail-closed outcome assertion still passes | red (6 failed) |
| b — revert body-listener id branch | 2 / 2 / 3 ✔ | red — killed | 49/32 — fail-closed BROKEN | red (1 failed) |
| a+b — combination row | 0 / 1 / 3 ✔ | red — killed | 43/38 | red (6 failed) |
| c — revert module-scope mount guard | 2 / 3 / 2 ✔ | red — killed | 55/26 — hazard realized | green (out of that file's scope) |
| d — re-add the guard the closeout commit removed | 2 / 3 / 4 ✔ | green | 81 / 0 / 81 — indistinguishable from control | green (6) |
Positive controls are green on all three oracles, and three of four mutants turn the gate red with the
intended behavioural mismatch, so the kills are attributable and the harness is not dead.
Row b is the decisive one, and it is new evidence. With the latch present (probe latch=2) and only the
body listener's stylesheet branch removed, on a real HTTP 404:
renderComplete="true" role=null transcriptDom=1 sheet=true cascades=false latch=true
timeline: init@5 | error:transcript-stylesheet@8 | renderComplete=true@…
latch=true — window.__transcriptStyleFailed was set — and the document still rendered and stamped
itself complete. So the body script's if (window.__transcriptStyleFailed) showLoadError(); reader ran
before the flag was set: Chromium unblocked the parser, ran the body script, and only then dispatched the
error task. Row a says the same thing from the other side (remove the latch, nothing changes). Together they
are a mutation-based proof, not a failed reproduction, that in this environment the comment in
document-index.html —
"A stylesheet failure that settles while the parser is still blocked on the
<link>is dispatched before
the listener above exists, so the<head>latch is the only record of it."
— is false: the latch is a record nobody reads in time. The equivalent comment block in the <head> and the
one in scripts/tests/export-transcript-document-template.test.js make the same claim.
Row d validates the closeout commit's removal. Re-adding
if (document.body.dataset.renderComplete !== 'error') around the rAF stamp produces byte-identical outcomes
across all 81 real-HTTP assertions and a green gate — the axis is unobservable, so the guard was redundant and
deleting it (with the matching design-doc bullets in both languages) was the right convergence. Per the
unpinned-axis rule: the suite cannot tell head from head-plus-guard, and that is the correct answer here
rather than a coverage gap, because row d shows there is nothing to cover.
Bounded — what does NOT hold. I looked for the reachable ordering in which the transcript renders
unstyled and is stamped complete at head, because showLoadError() early-returns on
if (document.body.dataset.renderComplete) return; while the rAF now stamps 'true' unconditionally. It does
not occur at head: across 9 rungs — real 404 at 0/800/2500 ms, real SRI mismatch, empty 200 body,
renderer 404, and the maximum-size document at 800 ms — every CSS-failure rung ends at
renderComplete="error", role="alert", transcriptDom=0, and the rAF never stamps 'true'. The hazard
only appears in row c, i.e. when the remaining mount guard is removed. So the closeout commit removed the
redundant half and kept the load-bearing half. Parser-blocking is what makes the ordering safe: the
renderer <script> cannot execute until the stylesheet settles, so the failure always precedes the mount
guard's check.
Severity is a Suggestion, unchanged: the latch is additive, fail-safe, cannot make anything worse, and its
static test pins the properties that could actually be wrong (position, nonce, capture phase, id agreement).
What is actionable is the prose.
Honest limits on the negative. All of this is headless Chromium on Linux in this container. Task ordering
between "parser unblocks" and "resource error dispatched" is not specified, so a different Chromium version,
a real (non-loopback) network with DNS/TLS latency, or a slower device could plausibly dispatch the error
before the body script runs — in which case the latch would be load-bearing and the comment right. If the
author has such a reproduction, recording it turns six static assertions into a demonstrated mechanism; if
not, the three comments overstate what is proven.
Suggested minimal change (prose only — not applied)
Reword the three comments to what is measured: the latch records a stylesheet failure that settles before the
body script runs, and in this environment (headless Chromium, loopback and real 404/SRI failures, delays to
2500 ms, maximum-size document) the body listener is what actually closes the document — the latch is
defence-in-depth for an ordering not observed here. No source change is implied.
F3 (Nit) — one deleted comment carried rationale that survives nowhere
The delta deleted three comment blocks. Two are fine: the transcript-css-entry.mjs rationale (both
separators; why the transcript\.js$ tail must not match the barred web-shell/dist/index.js) survives
verbatim in scripts/tests/transcript-css-entry-filter.test.js, and the copy_bundle_assets.js "warning, not
a throw — that script is the release gate, this one also serves --cli-only dev bundles" rationale survives
in scripts/prepare-package.js:78 and copy_bundle_assets.js:584.
The third does not. build.mjs lost 35 lines explaining the delegation knob, including the specific reason it
is deliberately not wired into CI:
"the envelope would announce the delegated identity while the asset running in the page announces its own,
anddocument-main.tsxfails closed on exactly that mismatch."
The replacement says only "CI serves the local assets and intentionally leaves delegation disabled" — the
instruction survives, the mechanism does not, and docs/verification/export-renderer-delegation-mermaid/README.md
describes the mismatch symptom without tying it to CI. Trimming is consistent with the repo's comment policy;
this one line of why is the part worth keeping.
Vacuity and gate liveness
- The repo browser gate is live and reaches the code under test: control green, three of four mutants red
with the intended mismatch, and each row's reachability probe confirms the mutation landed in the artifact
the gate consumes. (Round 1's first matrix was voided by exactly this; the probe is why this one is not.) - The real-HTTP harness is live and self-audited. Its first version had two defects I found and fixed
before believing any result:MutationObserver.observe(document.documentElement)throws at document-start
(documentElement is still null), and that TypeError dispatched a window error — which the page's own
showLoadErrorlistens for, so the harness was injecting a fake fail-closed cause into every rung. Two
validity controls now run per rung and are counted: no self-inflicted window error, and the recorder
observed therenderCompletetransition. A third assertion I had written — "stylesheet settles before the
renderer script loads" — compared againstInfinitybecause the<script>load event was never captured,
so it could not fail; it is replaced by two falsifiable ones (the latch recorded the failure; the rAF never
stamped'true'). - The assertions the closeout commit added to
html.test.tsare not vacuous, and the interesting part is
what the first two mutants showed. Dropping the nonce from the stylesheet<link>kills the new nonce
assertion with the intended mismatch. Removing the link entirely, and separately making the CSS integrity a
non-digest, both go red — but on earlier assertions (toContain(css-url)and the pre-existing
toHaveLength(2)), so neither proves anything about the new lines. A fourth, precisely-targeted mutant
does: moving the CSS digest from the stylesheet<link>to the favicon<link>leaves exactly 2 valid
sha384 digests on the page (sotoHaveLength(2)stays green) and kills only
expect(stylesheetLink).toMatch(/integrity="sha384-…/). That is the security property the new assertion
exists to bind — the digest must be on the stylesheet element, not merely somewhere in the document — and it
is pinned. - The static template test is live: row a turns all 6 cases red; row b turns 1 red.
- Packaging tests are live:
scripts/tests/package-assets.test.js37 tests green, covering the
all-or-nothing copy branch, the "name the missing stylesheet" warning, andpreparePackageexiting 1 when
the published CSS is absent.
Not covered
scripts/tests/install-script.test.jsstill cannot run here, and the cause is environmental — proven by
A/A, not assumed. It throws at import from its own guard (`zip`/`unzip` missing on a CI host;
CI=true,command -v zipempty, only/usr/bin/unzippresent, and the container is not root so
apt-get install zipis refused withPermission deniedon the dpkg lock). The identical file at base
005fc97bfails the same way (Test Files 1 failed (1),Tests no tests), and the guard is untouched by
the PR (0 matches forzipAvailableingit diff HEAD^1..HEAD). Consequence: the PR's new assertion
that standalone archives excludelib/export-transcript-document.csswas not executed. I verified the
intent statically only — the CSS is a member ofDIST_NPM_PACKAGE_ONLY_ENTRIES, which is the exact predicate
the copy loop consults atcreate-standalone-package.js:380— but that set is not exported, so it cannot be
asserted without running the packager, which needs a full dist andzip. Weaker than a run; labelled as such.- Per-commit attribution is still partial. The depth-2 checkout makes
git rev-list HEAD^1..HEAD^2return
1 commit while the snapshot records 7. This round I recovered the history with a read-only anonymous
git fetch --depth=12 origin pull/11485/head, which let me isolate the exact delta
(328feb43..30d8989a, 2 commits, 12 files) and scope new probes to it — but I verified the aggregate
HEAD^1..HEADdiff, and the two delta commits (77537e46,30d8989a) were exercised as one change, not
individually. The five earlier commits were not re-attributed either. - Repo-wide
typecheck,lintandprettierwere not run — the PR's own CI covers them and no A/B needed
the numbers. - No Windows host.
TRANSCRIPT_CSS_ENTRY_FILTERaccepts both separators and rejects
web-shell/dist/index.js(2 unit tests green), and I confirmed the POSIX branch really fires (the extraction
happened, sha256-identical). The[\\/]class remains unexercised end-to-end. - Fail-closed was measured in one engine. Headless Chromium on Linux. Real wire failures this time
(loopback HTTP, no routing), but not Safari/Firefox and not a real CDN with DNS/TLS latency — which is
exactly the regime where F2′'s ordering could differ. release-vscode-companion.ymlsequencing was read, not run, and the missing CSScmpwas not exercised.- The maximum-document rung used the format's own envelope (
maxBlocks=1000, ~8 MB JSON, mirroring the
gate'screateMaximumDocument), not a larger synthetic one;maxEnvelopeBytesis 32 MB and I did not probe
between the two.
Methodology
Everything ran in the CI verify container (node:22-bookworm, Node v22.23.2, non-root) at the merge commit
e6bf2b76, with npm ci and npm run build already completed. Scratch worktrees tmp/base-tree (HEAD^1)
and tmp/head-tree (HEAD) hosted the A/B; both were given a symlink to the production
packages/web-templates/node_modules so esbuild resolved to 0.21.5 on both arms, and both were removed
afterwards. ab-harness.mjs (29 assertions) measures shipped bytes, gzip −9, both SRI digests against the
bytes that actually ship, URL/version agreement, an independent re-implementation of the CSS lift compared by
sha256, the url() census, and the budget ratchet. failclosed-harness.mjs (81 assertions) serves the real
rendered document — produced by the compiled packages/cli/dist formatter and the real
createExportTranscriptDocumentV1, with both asset URLs rewritten to the loopback server and their integrity
attributes left intact so Chromium performs real SRI verification — and drives 9 rungs in headless Chromium
with an addInitScript timeline recorder; no Playwright request routing is installed anywhere.
delegation-harness.mjs (26 assertions) drives all 2³ environment combinations through the real build.mjs
in tmp/head-tree, pins each throw to its intended guard by message rather than exit code, and uses synthetic
delegated digests so "the env value reached the document" cannot be satisfied by the local build's own.
mutation-driver.sh and vacuity-driver.sh mutate the main tree, run the full workspace build, probe both
consumed artifacts, run the oracles, and git checkout -- afterwards; git status --porcelain is empty at the
end. matrix-assertions.mjs (29 assertions) re-reads the saved logs and encodes every expected red as a pass.
Raw per-cell output lives in logs/ (build-*-esbuild0215.log, ab.log, failclosed-head.log,
delegation.log, gate-head-control.log, unit-cli.log, unit-scripts.log,
aa-install-script-base.log, mutation-matrix.log, matrix-assertions.log, vacuity-*.log) and per-row
logs in mutants/. Live unpkg and npm-registry probes were anonymous HTTPS reads; no GitHub API call was made
and nothing was posted.
Flakiness gate log
integration test, out of gate scope: integration-tests/chat-transcript-document.test.ts
rounds=5 files=5 skipped=1
file packages/cli/src/ui/utils/export/formatters/html.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/utils/export/formatters/html.test.ts
file scripts/tests/export-transcript-document-template.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/export-transcript-document-template.test.js
file scripts/tests/install-script.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/install-script.test.js
file scripts/tests/package-assets.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/package-assets.test.js
file scripts/tests/transcript-css-entry-filter.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/transcript-css-entry-filter.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/ui/utils/export/formatters/html.test.ts: PPPPP
scripts/tests/export-transcript-document-template.test.js: PPPPP
scripts/tests/install-script.test.js: FFFFF
scripts/tests/package-assets.test.js: PPPPP
scripts/tests/transcript-css-entry-filter.test.js: PPPPP
verdict: consistent-fail
summary: 1 of 5 changed test file(s) failed identically in every round — deterministic, so CI owns that signal
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 1 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 1 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 1 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 04:24:37
�[2m Duration �[22m 606ms�[2m (transform 144ms, setup 21ms, collect 0ms, tests 0ms, environment 0ms, prepare 126ms)�[22m
round 1 · scripts/tests/package-assets.test.js: P (exit 0)
round 1 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
round 2 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 2 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 2 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 2 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 04:24:50
�[2m Duration �[22m 622ms�[2m (transform 157ms, setup 28ms, collect 0ms, tests 0ms, environment 0ms, prepare 75ms)�[22m
round 2 · scripts/tests/package-assets.test.js: P (exit 0)
round 2 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
round 3 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 3 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 3 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 3 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 04:25:03
�[2m Duration �[22m 707ms�[2m (transform 141ms, setup 22ms, collect 0ms, tests 0ms, environment 0ms, prepare 161ms)�[22m
round 3 · scripts/tests/package-assets.test.js: P (exit 0)
round 3 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
round 4 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 4 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 4 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 4 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 04:25:18
�[2m Duration �[22m 775ms�[2m (transform 142ms, setup 21ms, collect 0ms, tests 0ms, environment 0ms, prepare 221ms)�[22m
round 4 · scripts/tests/package-assets.test.js: P (exit 0)
round 4 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
round 5 · packages/cli/src/ui/utils/export/formatters/html.test.ts: P (exit 0)
round 5 · scripts/tests/export-transcript-document-template.test.js: P (exit 0)
round 5 · scripts/tests/install-script.test.js: F (exit 1)
--- output tail · round 5 · scripts/tests/install-script.test.js ---
�[1m�[46m RUN �[49m�[22m �[36mv3.2.7 �[39m�[90m/__w/qwen-code/qwen-code�[39m
�[31m⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Suites 1 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m
�[41m�[1m FAIL �[22m�[49m scripts/tests/install-script.test.js�[2m [ scripts/tests/install-script.test.js ]�[22m
�[31m�[1mError�[22m: `zip`/`unzip` missing on a CI host; archive tests would skip.�[39m
�[36m �[2m❯�[22m scripts/tests/install-script.test.js:�[2m56:9�[22m�[39m
�[90m 54| �[39m spawnSync('unzip', ['-v']).error === undefined);
�[90m 55| �[39mif (process.env.CI && process.platform !== 'win32' && !zipAvailable) {
�[90m 56| �[39m throw new Error(
�[90m | �[39m �[31m^�[39m
�[90m 57| �[39m '`zip`/`unzip` missing on a CI host; archive tests would skip.',
�[90m 58| �[39m );
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m (1)�[39m
�[2m Tests �[22m �[2mno tests�[22m
�[2m Start at �[22m 04:25:36
�[2m Duration �[22m 915ms�[2m (transform 221ms, setup 45ms, collect 0ms, tests 0ms, environment 0ms, prepare 185ms)�[22m
round 5 · scripts/tests/package-assets.test.js: P (exit 0)
round 5 · scripts/tests/transcript-css-entry-filter.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Reviewed the latest sandbox report against
No code change is needed from this report. All 15 inline review threads are resolved and the required CI checks pass; only the automated review job is still pending. |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings.
Approval blockers: none.
Scope: source code only. NOT reviewed — design doc content (informational, 2 files) · Chinese translation of design doc.
Ran: static analysis of all 18 files; cross-file on the CSS extraction pipeline (build.mjs → transcript-css-entry.mjs → document-index.html → document-main.tsx) and the packaging chain (copy_bundle_assets.js → prepare-package.js → create-standalone-package.js). Rung 3 not run (no browser environment available); integration tests cover it.
Checked:
- CSS extraction regex (
build.mjsextractTranscriptCss): traced^const __qwenWebShellCss=("(?:[^"\]|\\.)*");\nthrough escaped-quote and escaped-backslash edge cases — the(?:[^"\]|\\.)*alternation correctly pairs every backslash with the next character, so a CSS string ending with a literal\is captured without leaking the closing quote. Both the CSS-constant guard and the injection-line guard throw on shape mismatch, so a web-shell build change fails the export build rather than shipping a double-injected stylesheet. - Template placeholder chain: the two new placeholders
__DOCUMENT_RENDERER_CSS_URL__and__DOCUMENT_RENDERER_CSS_INTEGRITY__are both in the residual-placeholder guard regex, so a dropped.replace()fails the build. - CSS
<link>placement:<link>is after inline<style>indocument-index.html, so the component sheet wins equal-specificity ties by document order. - Fail-closed path: three layers — head latch (capture-phase error listener set before the
<link>, handles the Chromium parser-blocking window), body error listener (catches CSS failure after registration),document-main.tsxguard (data-render-complete !== 'error'prevents unstyled render). The latch is correctly record-only (nodocument.bodyaccess in<head>). - Delegation env vars:
QWEN_EXPORT_RENDERER_CSS_INTEGRITYvalidation mirrors the JS integrity check; the "set both or neither" guard is extended to require all three together. - Packaging consistency: all-or-nothing copy in
copy_bundle_assets.jsmatches the all-required semantics ofverifyBundleArtifacts; the warning names the specifically-missing asset. - Filter cross-platform:
TRANSCRIPT_CSS_ENTRY_FILTERuses[\\/]and the$anchor ontranscript\.jsto avoid matching the barredweb-shell/dist/index.js.
Cross-check vs existing reviews:
The qwen-code-ci-bot review at head 3b63662b filed 2 Criticals (forward-slash filter, error event not reaching the listener) and ~12 Suggestions. Both Criticals are fixed at current head 30d8989a — the filter now uses [\\/] and the head latch closes the parser-blocking window. qqqys dismissed their review at d54fcd0f confirming both Criticals were resolved. The author's follow-up commits (77537e46c5 and 30d8989a) addressed the remaining Suggestions (copy semantics, delegation docblock, test assertions). I confirmed each fix against the current code.
No existing finding at current head stands as an unresolved blocker.
Not covered: Windows runtime behavior (unit test pins both separators but runs on one OS) · real browser execution (integration tests cover it; I cannot run them).
Reviewed with AI assistance.
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Approved at head 30d8989a.
Required CI is green at this commit, and I confirmed the part that matters was actually executed rather than skipped: the web-shell E2E Smoke (ubuntu-latest, Node 22.x) job shows Run transcript document browser gate = success with --retry=0, so the real headless-Chromium gate ran the new stylesheet cases at this head. Test, Lint & Static and Integration Tests (no-AK) are green too; only review-pr is still running.
Both Criticals from the earlier REQUEST_CHANGES are fixed, each with the remedy the finding named:
- The Windows path separator (R1-1). The filter is no longer a forward-slash-only literal buried in the build: it is
TRANSCRIPT_CSS_ENTRY_FILTER = /web-shell[\\/]dist[\\/]transcript\.js$/intranscript-css-entry.mjs, imported bybuild.mjs, and pinned by a test that asserts both/repo/packages/web-shell/dist/transcript.jsandC:\\repo\\packages\\web-shell\\dist\\transcript.jsmatch whiledist/index.jsdoes not — so the widened class cannot regress into lifting the wrong stylesheet, and the POSIX side the browser gate depends on is asserted as well. This is the cheap pin the finding asked for, and it runs wherever the scripts suite runs. - The stylesheet fail-closed race (R1-2). The listener now exists in
<head>before the<link>, in capture phase, and only latcheswindow.__transcriptStyleFailed— with a comment explaining thatdocument.bodydoes not exist yet at that point. The body script acts on the latch, anddocument-main.tsxrefuses to mount whenrenderComplete === 'error', so a fast CSS failure can no longer produce an unstyled transcript stamped as complete. The new browser-gate case aborts the stylesheet request and asserts the same alert page a missing renderer produces, and the cascade is still proven by an oracle that only the split stylesheet can satisfy (the KaTeXfont-family), with the allowed style requests pinned to exactly the one CSS URL.
The packaging side moved in lockstep, which is where this class of change usually breaks: copyBundleAssets copies the CSS only when both siblings exist and now names the missing paths in the warning, prepare-package's verifyBundleArtifacts requires the CSS so a published package cannot ship without it, writeDistPackageJson ships it, and create-standalone-package lists it beside the renderer as npm-package-only. The delegation contract validates all three env inputs together (IDENTITY ↔ INTEGRITY ↔ CSS_INTEGRITY, plus format checks), which closes the "one without the other always fails closed" hole. The budgets are re-ratcheted to the JS alone with the reasoning written down, including why the byte cap is the weaker of the two guards and why the shape-keyed throw is the only thing standing between a changed injectCssModules and a document that both links and injects 2.3 MB of CSS.
No new Critical found. Stated limits I share with the author: Test (windows-latest) never runs on a pull request, so the Windows build proof arrives with the merge queue or the nightly, and total transferred bytes are unchanged — this moves them off the parse/compile path.
Stale CHANGES_REQUESTED, raised at 3b63662 on 2026-09-09T18:19Z; head is now 30d8989 (two further pushes: d54fcd0, 328feb4). Both round-1 Criticals were fixed with cited SHAs and measured evidence in the threads (R1-1 esbuild onLoad filter made path-separator agnostic, fixed in 726695d; R1-2 stylesheet fail-closed latch moved into before the , fixed in c57203f), and the bot's own round-2 re-review at d54fcd0 did not re-mint either finding. 15/15 review threads resolved; remaining R1-4/R1-5/R1-23 are Suggestion-level follow-ups explicitly carried, not silently dropped. CI green at 30d8989 and chiga0 APPROVED at that head. Dismissing to unblock.
测试报告结论结论：通过，改动已验证有效、可安全合入。 核心结论一句话：渲染器 JS（浏览器「下载+解析+编译」关键路径）从 4,136,297 B 降到 1,831,298 B（−56%），gzip 从 ~1.5 MB 降到 526 KB；抽出的 2,302,457 B CSS 变成独立、版本固定、SRI 校验、缓存一年的并行资产。导出文档在真实浏览器中完整渲染且样式表真实生效，缺失样式表时正确 fail-closed。 测试矩阵
关键断言（证明「不只是下载了、而是真用上了」）
完整报告见 |
Two both-side changes, resolved without rewriting history: - packages/web-templates/src/export-html/build.mjs: main (#11372) raised the document-runtime budget to 4,200,000 / 4,300,000 for a combined JS+CSS measurement of 4,133,282 bytes at c3023b3. This branch lifts the ~2.3 MB component stylesheet out into export-transcript-document.css, so the JS-only budget stands (1,870,000 warning / 1,930,000 max against a measured 1,833,894 bytes of JS) and the comment now records both measurements instead of silently dropping main's. - scripts/copy_bundle_assets.js: main added the musl/glibc filter over the @opentui/core-* native libraries inside copyOpenTuiAssets; this branch made the HTML export renderer copy all-or-nothing over the JS and the new CSS in a different function. Three-way merge is clean, both changes kept. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
5773c84
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Self-review at head 30d8989a (GitHub does not let me approve my own PR). No blocking finding. One follow-up worth filing, and one earlier claim I want to retract before someone acts on it.
What I verified rather than assumed
- The fail-closed path is genuine, single-latching and race-free. The
<head>latch script is registered capture-phase before the<link>(document-index.html:25-43), the body script consumeswindow.__transcriptStyleFailed, and a second capture listener independently matchesevent.target.id === 'transcript-stylesheet'for a failure that settles later — so neither window can miss it.showLoadErrorearly-returns on any existingdata-render-complete, so it cannot latch twice. Script execution parser-blocks on the pending stylesheet, and the mount guard atdocument-main.tsx:340then skipscreateRootentirely. - Digest and artifact cannot diverge.
build.mjs:361-363hashesextractedTranscriptCss.cssandbuild.mjs:423-426writes that identical string; the JS path is symmetric. Both URLs derive from the oneexportTranscriptRendererVersion, and delegation is all-three-or-none via the two XOR throws plus format validation. - Windows.
TRANSCRIPT_CSS_ENTRY_FILTER = /web-shell[\\/]dist[\\/]transcript\.js$/matches both separators,scripts/tests/transcript-css-entry-filter.test.jspins both plus the negativeweb-shell/dist/index.js, andbuild.mjsfails the build if the filter never fired. TheonLoadcallback compares file contents, not paths. - Packaging, all three shapes. npm tarball: copied all-or-nothing (
copy_bundle_assets.js:584-594), required byprepare-package.js:82(console.error+process.exit(1)), published via thefilesentry at:334— a missing asset fails loudly at the release gate. Standalone: deliberately excluded viaDIST_NPM_PACKAGE_ONLY_ENTRIES(create-standalone-package.js:119), tested at:380beforeisAllowedDistEntry, identical to the pre-existing treatment of the JS. No third shape exists. The four script tests read the asset back and compare content, assert thefilesentry, assert the JS-present/CSS-absent branch names the missing path, and assertpreparePackagethrows through a mockedprocess.exit(1)— they assert the copy happened, not that code ran.
Retracting a claim about the renderComplete rAF guard
77537e46c5 broadened that guard's comment and 30d8989a33 removed the guard, which reads like a closeout commit deleting defence-in-depth. It is not: base (fbb877a4) has no guard either — git show fbb877a4:…/document-main.tsx line 245 is the same bare document.body.dataset.renderComplete = 'true'. git log -S"renderComplete !== 'error'" shows the string added by 3b63662b98 and removed by 30d8989a33, so the PR nets to base on that line. The guard that is load-bearing for this PR is the new mount-time one at document-main.tsx:340, which prevents createRoot at all when the stylesheet failure already latched 'error'. An async error landing after mount and being overwritten by the rAF is base behaviour, not a regression here. Filing that as a defect would have been wrong.
The one follow-up
Inline on build.mjs:360. Short version: .github/workflows/release-vscode-companion.yml:106-111 fetches export-transcript-document.js from unpkg and cmps it against the local build. This PR lifted ~2.3 MB of CSS out of that byte-compared artifact into a second published file whose SRI is baked from the local build, and nothing compares the published copy. That file is not in this PR's 18 changed files, so it is a follow-up rather than a change I would make here.
CI at 30d8989a: Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke, Desktop Shell (both), TUI parity and the OpenTUI gate all pass; only review-pr is pending. The CHANGES_REQUESTED on this PR is the round-1 review at 3b63662b98, six commits behind head — the round-3 ledger at 328feb43f8 records findings: [], and all 15 inline threads are resolved.
中文说明
在 head 30d8989a 上的自审（GitHub 不允许我批准自己的 PR）。没有阻塞项。 有一条值得开后续，以及一条我要在别人据此行动之前撤回的断言。
我核实过而非假定的部分
- 失败关闭路径是真实、单次锁存且无竞态的。
<head>里的锁存脚本以捕获阶段注册在<link>之前（document-index.html:25-43），body 脚本消费window.__transcriptStyleFailed，另有第二个捕获监听器独立匹配event.target.id === 'transcript-stylesheet'以覆盖较晚才确定的失败——两个窗口都不会漏。showLoadError在已有data-render-complete时提前返回，因此不会锁存两次。脚本执行会被待加载的样式表解析阻塞，随后document-main.tsx:340的挂载守卫直接跳过createRoot。 - 摘要与产物不会分叉。
build.mjs:361-363对extractedTranscriptCss.css取哈希，build.mjs:423-426写出的正是同一字符串；JS 路径对称。两个 URL 都派生自同一个exportTranscriptRendererVersion，委托则通过两处 XOR 抛错加格式校验做到「三者全有或全无」。 - Windows。
TRANSCRIPT_CSS_ENTRY_FILTER = /web-shell[\\/]dist[\\/]transcript\.js$/两种分隔符都匹配，scripts/tests/transcript-css-entry-filter.test.js钉住了两种分隔符以及反例web-shell/dist/index.js，且build.mjs在过滤器从未命中时让构建失败。onLoad回调比较的是文件内容而非路径。 - 打包，三种形态。 npm tarball：全有或全无地复制（
copy_bundle_assets.js:584-594），被prepare-package.js:82强制要求（console.error+process.exit(1)），并经:334的files条目发布——缺资产会在发布门禁处大声失败。standalone：通过DIST_NPM_PACKAGE_ONLY_ENTRIES刻意排除（create-standalone-package.js:119），在:380于isAllowedDistEntry之前被测到，与 JS 的既有处理一致。不存在第三种形态。四个脚本测试会读回资产并比对内容、断言files条目、断言「有 JS 缺 CSS」分支点名缺失路径、并断言preparePackage透过被 mock 的process.exit(1)抛错——它们断言的是复制确实发生，而不是代码跑过。
撤回一条关于 renderComplete rAF 守卫的断言
77537e46c5 拓宽了该守卫的注释，30d8989a33 删除了守卫，看起来像收尾提交删掉了纵深防御。并不是：base（fbb877a4）同样没有守卫——git show fbb877a4:…/document-main.tsx 第 245 行就是同一句裸的 document.body.dataset.renderComplete = 'true'。git log -S"renderComplete !== 'error'" 显示该串由 3b63662b98 加入、由 30d8989a33 移除，所以本 PR 在这一行的净变化等于 base。对本 PR 真正承重的守卫是新增的挂载期那一个（document-main.tsx:340），它在样式表失败已锁存 'error' 时根本不允许 createRoot。挂载之后才到达的异步错误被 rAF 覆盖，是 base 行为，不是此处的回归。把它当作缺陷上报会是错的。
唯一的后续项
见 build.mjs:360 的内联评论。简版：.github/workflows/release-vscode-companion.yml:106-111 从 unpkg 取 export-transcript-document.js 与本地构建做 cmp。本 PR 把约 2.3 MB 的 CSS 从这个被逐字节比对的产物里拆出来，变成第二个已发布文件，而它的 SRI 是从本地构建烘焙的——却没有任何东西比对已发布的那一份。该文件不在本 PR 的 18 个改动文件里，所以属于后续项，不是我会在这里改的东西。
30d8989a 的 CI：Test (ubuntu-latest, Node 22.x)、Lint & Static、Integration Tests (no-AK, No Sandbox)、web-shell E2E Smoke、Desktop Shell（两个）、TUI parity 与 OpenTUI 门禁全部通过，只有 review-pr 待完成。本 PR 上的 CHANGES_REQUESTED 是 head 之前六个提交的 round-1 评审（3b63662b98）——328feb43f8 上的 round-3 账本记录 findings: []，且 15 条内联线程全部已解决。
| @@ -301,6 +357,10 @@ const documentRendererUrl = `https://unpkg.com/@qwen-code/qwen-code@${exportTran | |||
| const documentRendererIntegrity = | |||
| rendererDelegateIntegrity ?? | |||
| `sha384-${createHash('sha384').update(documentJs).digest('base64')}`; | |||
| const documentRendererCssUrl = `https://unpkg.com/@qwen-code/qwen-code@${exportTranscriptRendererVersion.split('+')[0]}/export-transcript-document.css`; | |||
There was a problem hiding this comment.
[Suggestion — follow-up, outside this diff] Splitting the CSS out moved ~2.3 MB of published bytes out of the one artifact that gets byte-compared after release, and nothing took over the check.
.github/workflows/release-vscode-companion.yml:106-111 fetches https://unpkg.com/@qwen-code/qwen-code@${renderer_version}/export-transcript-document.js and cmps it against dist/export-transcript-document.js. At base that transitively verified the stylesheet too, because the CSS lived inside the compared JS. Now the CSS is published separately (prepare-package.js:334 files entry) while its integrity is baked here from the local build (:361-363, sha384 over extractedTranscriptCss.css) and written to dist/export-transcript-document.css at :423-426.
So if the published CSS ever differs from the CSS this build hashed — a stale dist/ from a previous build picked up by the copy step, a partial publish — every exported document for that version fails its <link integrity> check and fail-closes to the load-error alert for all readers, while the companion-release gate stays green. The failure mode is deliberately loud for the reader and deliberately silent for the release.
Two cheap options: add a second fetch + cmp for export-transcript-document.css in the same step, or compare the published file's sha384 against the baked __DOCUMENT_RENDERER_CSS_INTEGRITY__ value, which needs no local artifact. Not a change for this PR — the workflow file is not in its diff — but worth an issue so the split does not leave the new asset permanently unverified.
…-css One both-side change: packages/web-templates/src/export-html/build.mjs. main's #11421 added the import.meta guard (a new import plus the post-build findUnexpectedImportMeta throw) while this branch added the transcript CSS entry filter import at the same spot. Both imports are kept and the guard block is untouched. The budget constants stay at this branch's JS-only values (1,870,000 warning / 1,930,000 max against a measured 1,833,894 bytes of renderer JS), and the comment now also records main's pre-split combined measurement of 4,133,282 bytes that #11372 raised them for, so that history is not lost by the split. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings.
Approval blockers: none.
Re-review (Round 2). Prior review at head 30d8989a / base fbb877a4 found no blockers and approved. That review was dismissed by the two merge-main commits (5773c846, d7c36a18). The PR's own code is byte-for-byte identical between the two review heads (confirmed via compare API: all 18 PR files show +0/-0 between 30d8989a and d7c36a18). No new review-surface exists; the prior analysis stands.
New base context (f6540d19 vs fbb877a4):
The only commit in the merged-main range that touches this PR's files is cb24374d (fix(export): restore document runtime budget headroom #11372). That PR raised the budget constants in main — the prior CI-bot Critical about sequencing conflict. This PR already resolved it: its build.mjs carries DOCUMENT_RUNTIME_WARNING_BYTES = 1_870_000 / MAX_DOCUMENT_RUNTIME_BYTES = 1_930_000, budgeting the post-split JS-only renderer. The merge-main commit is clean (build.mjs unchanged between 30d8989a and d7c36a18). ✓
Cross-check vs prior reviews (frozen findings above):
- CI-bot R1-1 (forward-slash filter): fixed at
30d8989a([\\/]separator class). Verified present at current head. ✓ - CI-bot Critical (sequencing with #11372):
#11372landed on main, this PR merged it cleanly. ✓ qqqys(dismissed atd54fcd0f): confirmed both Criticals resolved. Consistent with current head. ✓- No new review content since my last submission.
Not covered: Windows runtime behavior (separator test is unit-level only); real browser execution (integration tests cover it; browser unavailable here).
Reviewed with AI assistance.
|
Released in v0.23.3. |
6 participants
What this PR does
Splits the export transcript renderer's embedded component stylesheet into a separate, version-pinned, SRI-protected asset. The exported document now loads
export-transcript-document.cssfrom unpkg via a nonce-bearing<link>in parallel with the renderer JS, instead of carrying a ~2.3 MB CSS string literal inside the JS bundle. The transform happens entirely in the export build through an esbuild plugin that lifts the injected CSS out of the web-shell transcript entry and hands the bundler a stub, so web-shell source and runtime behavior are unchanged. The document's fail-closed load-error path is extended to the stylesheet: a missing CSS asset now shows the same load-error page as a missing renderer.Why it's needed
The renderer asset was 4,136,297 bytes at 0.23.2, and 56% of those bytes were the inlined component CSS string. Browsers had to download, parse, and compile all 4.1 MB of JS before a transcript could render, and the build's byte budget was within ~60 KB of its hard cap. After the split the renderer JS is 1,831,301 bytes (the CSS moves to a 2,302,457-byte asset that loads, parses, and caches separately). This does not reduce total downloaded bytes — it moves them off the parse/compile critical path and re-ratchets the budget to the JS alone.
Reviewer Test Plan
How to verify
Rebuild the web-templates package and confirm the build prints the split sizes and writes the CSS asset:
Run the focused unit tests:
Run the transcript browser gate, which opens a real exported document in headless Chromium and now fulfils the stylesheet request from the built asset:
The gate asserts the stylesheet
<link>is the only stylesheet request, that its SRI check passes (link.sheet !== null), and that the component CSS actually cascades (the KaTeX font-family on a rendered formula comes only from that stylesheet). A new case aborts the stylesheet request and asserts the document fails closed with the same "Unable to load this chat export" page as a missing renderer.Evidence (Before & After)
export-transcript-document.jsexport-transcript-document.css__qwenWebShellCssliteral in JSLocal integration run:
chat-transcript-document.test.ts— 6 passed (includes the new "fails closed when the CDN stylesheet is unavailable" case and the stylesheet-applied assertion).Tested on
Environment (optional)
Linux, Node 24, local
npm run build --workspace=@qwen-code/web-templates+npx vitest(Playwright Chromium headless shell).Risk & Scope
export-transcript-document.css(added to the package files and the standalone-exclusion list). Exports from an older published version that predates the split will 404 the CSS and fail closed until a new release publishes it.Design docs: English · 简体中文
Linked Issues
Fixes #11478
中文说明
本 PR 做了什么
把导出 transcript 渲染器内嵌的组件样式表拆分为独立的、版本固定、带 SRI 校验的资产。导出文档现在通过带 nonce 的
<link>从 unpkg 并行加载export-transcript-document.css，而不是把约 2.3 MB 的 CSS 字符串字面量塞进 JS bundle。转换完全发生在导出构建中：通过一个 esbuild 插件把注入的 CSS 从 web-shell 的 transcript 入口抽出、并把剩余部分作为 stub 交给打包器，因此 web-shell 的源码与运行时行为保持不变。文档的 fail-closed 加载失败路径扩展到样式表：CSS 资产缺失时显示与渲染器缺失相同的加载错误页。为什么需要
渲染器资产在 0.23.2 为 4,136,297 字节，其中 56% 是内联的组件 CSS 字符串。浏览器必须先下载、解析、编译全部 4.1 MB 的 JS 才能渲染，而构建的体积预算距离硬上限只剩约 60 KB。拆分后渲染器 JS 为 1,831,301 字节（CSS 移到 2,302,457 字节的独立资产，单独加载、解析、缓存）。这不减少总下载字节，只是把它们从解析/编译关键路径上移走，并把预算重新收紧到「仅 JS」。
评审测试计划
如何验证
重新构建 web-templates 包，确认构建打印拆分后的体积并写出 CSS 资产：
运行聚焦单元测试：
运行 transcript 浏览器门禁（在无头 Chromium 中打开真实导出文档，并用构建出的资产满足样式表请求）：
门禁断言样式表
<link>是唯一的样式表请求、其 SRI 校验通过（link.sheet !== null）、组件 CSS 真正级联生效（渲染公式上的 KaTeX font-family 只来自该样式表）。新增用例中止样式表请求，断言文档以与缺失渲染器相同的「Unable to load this chat export」页面 fail-closed。证据（Before & After）
export-transcript-document.jsexport-transcript-document.css__qwenWebShellCss字面量本地集成运行：
chat-transcript-document.test.ts— 6 个通过（含新增的「CDN 样式表不可用时 fail-closed」用例和「样式表已应用」断言）。测试环境
运行环境（可选）
Linux、Node 24、本地
npm run build --workspace=@qwen-code/web-templates+npx vitest（Playwright Chromium headless shell）。风险与范围
export-transcript-document.css（已加入包文件与 standalone 排除列表）。早于本次拆分的旧版本导出在 CSS 发布前会 404 并 fail-closed。设计文档：English · 简体中文
关联 Issue
Fixes #11478