Skip to content

fix(export): load transcript renderer from unpkg - #11035

Merged
yiliang114 merged 4 commits into
codex/remove-webuifrom
codex/fix-11031-html-export-cdn
Sep 5, 2026
Merged

fix(export): load transcript renderer from unpkg#11035
yiliang114 merged 4 commits into
codex/remove-webuifrom
codex/fix-11031-html-export-cdn

Conversation

@yiliang114

@yiliang114 yiliang114 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

HTML exports now keep only transcript data, styles, and a small bootstrap in the file. The complete Web Shell transcript renderer, including React and ReactDOM, is built as one browser-ready script, shipped at the root of the matching @qwen-code/qwen-code npm package, and loaded from an exact-version unpkg URL.

The generated HTML includes a SHA-384 Subresource Integrity value computed over the final published JavaScript bytes. The script remains nonce-gated by CSP, and a missing, modified, or incompatible renderer fails closed with a visible error instead of a blank page.

The renderer is deliberately excluded from standalone archives and GitHub release assets. Publishing the existing npm package is the only additional distribution path; no OSS synchronization is required.

Why it is needed

The current inline architecture makes every exported HTML file about 19.5 MB, even for an empty or one-message session. The renderer is application code shared by every export, so duplicating it into each session artifact makes exports unnecessarily expensive to store and share.

This restores the earlier unpkg-based distribution model while keeping the current Web Shell renderer. One fully bundled classic script avoids an import map and extra runtime dependency requests, and exact npm versions plus SRI prevent an existing export from silently changing behavior.

Reviewer Test Plan

How to verify

  1. Run Qwen Code from this branch and create a short session.
  2. Run /export html <directory>.
  3. Confirm the generated file is small and references only https://unpkg.com/@qwen-code/qwen-code@<exact-version>/export-transcript-document.js for executable code.
  4. Open it with the matching renderer asset available and verify the transcript, collapse/expand controls, and theme toggle.
  5. Block or modify the renderer response and verify the page shows a clear load error.
  6. Build and prepare the npm package, then confirm export-transcript-document.js is present at the package root and absent from standalone archives.

Evidence (Before & After)

Before: an empty export was 19,526,066 bytes because the renderer was inline.

After: the generated template is 6,606 bytes. The shared renderer is 19,520,274 bytes and is downloaded once through unpkg rather than copied into every exported session. The prepared npm tarball includes the renderer at the exact path used by the HTML.

Tested on

OS Result
macOS ✅ Full build, full typecheck, lint, formatter tests, package/standalone tests, npm pack inspection, and Chromium browser integration
Linux ⚠️ Covered by automated tests; not manually exercised
Windows ⚠️ Covered by automated tests; not manually exercised

Risk & Scope

  • Exported HTML requires network access to unpkg.
  • The npm package grows by about 19.5 MB unpacked; each exported HTML shrinks by the same amount, and the CDN asset is reusable across exports of the same version.
  • The first production release containing this change still needs a smoke test against its real unpkg URL. Source builds whose version has no matching published renderer fail closed by design.
  • Transcript validation, content-derived renderer identity, SRI, document-content network restrictions, and rendering behavior remain enforced.

Linked Issues

Fixes #11031

This is stacked on #9812 and should be retargeted to main after #9812 merges.


中文说明

导出的 HTML 现在只内联会话数据、样式和小型启动代码,约 6.6 KB。完整的 React + Web Shell renderer 作为一个浏览器脚本随同版本的 @qwen-code/qwen-code npm 包发布,并从精确版本的 unpkg URL 加载;最终脚本字节通过 SHA-384 SRI 校验。

OSS 发布和同步改动已经全部撤销。renderer 不进入 standalone 或 GitHub release asset,只复用现有 npm 发布流程。首个包含本改动的版本发布后,仍需对真实 unpkg URL 做一次 smoke test。

@yiliang114

yiliang114 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

E2E test report

✅ Re-ran the final 216b42b revision through the real tmux-driven Qwen Code flow and Chromium.

  • Started node dist/cli.js inside tmux, sent a prompt, then ran /export html
  • Exported HTML: 7,725 bytes; release renderer: 19,520,274 bytes and not inlined
  • Verified the only renderer URL is https://unpkg.com/@qwen-code/qwen-code@0.23.0/export-transcript-document.js
  • Verified crossorigin="anonymous" and SHA-384 SRI (sha384-pjxw3ovPqTWJZiXytFC5Wl5kyTuLS6Fjl3mfykOmnPTEh7DF5a2A7ONL0yRtLF/a) against the final renderer bytes
  • Verified there are no OSS, jsDelivr, import-map, or extra Web Shell CDN references
  • In Chromium, intercepted that exact pre-release unpkg URL with the exact local release asset and verified the transcript, metadata, collapse/expand, and theme switching
  • Aborted that exact request in a second Chromium run and verified the export fails closed with the explicit load-error page

The referenced npm version does not contain the asset until the first release with this PR. Intercepting only that exact URL validates the final HTML/CSP/SRI/runtime contract without pretending the unpublished CDN object already exists. A real-unpkg smoke test remains a release-time check.

Normal path:

tmux HTML export rendered with the exact release asset

Renderer unavailable:

HTML export explicit renderer load error

中文

✅ 已在 tmux 中真实启动 Qwen Code、发送 prompt 并执行 /export html。导出的 HTML 为 7,725 bytes,不再内联约 19.5 MB 的 renderer;Chromium 已验证正常渲染、元数据、折叠/展开、主题切换,以及 renderer 无法加载时的明确错误页。

由于目标版本尚未发布,测试仅拦截精确的 unpkg URL,并返回本地构建的同一发布产物。首个包含本改动的 npm 版本发布后,再对真实 unpkg URL 做一次 smoke test。

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR — the problem here is real and unusually well quantified, so the gate passes. A few things to flag before the code review.

Template — substantively complete, with two heading deviations from .github/pull_request_template.md: ## Why instead of ## Why it's needed, and ### Evidence instead of ### Evidence (Before & After) with no separate ### Tested on heading (the OS/status table is there, just nested under Evidence). Everything a reviewer needs is present, so I'm not gating on the wording — worth aligning next time you touch the body.

Problem — observed, not theoretical. #11031 carries byte counts (19,526,066 raw / 4,772,314 gzip against a 311,990-byte earlier export, ~62.6x), a real one-turn TUI file at 19,527,461 bytes, and four-step reproduction. The renderer is application code being duplicated into every session artifact; that is a straightforward defect, and the issue also asks for an automated size budget, which this PR adds.

Direction — aligned with what the linked P1 issue explicitly asks for (transcript data plus a small bootstrap, renderer and React from immutable version-pinned URLs). Two reasons I'm still flagging it rather than waving it through:

  • It touches release packaging (prepare-package.js's release gate, the npm files list, standalone archive exclusions) and changes the exported document's runtime contract from self-contained to network-dependent. Those are the sensitive areas where I escalate rather than decide alone.
  • It deliberately relaxes a documented security invariant. chat-transcript-contract-prevalidation.md §10.9 was titled "CSP 与零网络" and required that opening a local HTML produce no unregistered subrequest; this PR retitles it "CSP 与登记网络" and rewrites the clause to permit the version-bound renderer and React runtime. Updating the doc in the same PR is the right way to do that — I just want a human to own the decision, since the export contains the full session transcript.

The PR is also stacked on #9812, which is still open, and targets codex/remove-webui rather than main. It can't be judged as a mergeable unit yet, and — see Stage 2 — that base choice is why no CI ran.

Size — no Stage 0 core path is touched, so the two-tier core gate doesn't apply (and as a maintainer-authored PR it would be exempt regardless). For context: 141 non-test lines (122 build/packaging + 19 docs), 153 test lines, 0 generated/schema lines, across 12 files. Well under every threshold.

Approach — the scope feels right and I don't see anything to cut; the diff is confined to the export build, the template, the packaging chain, and the tests that cover them. No drive-by refactors. One thing I'd genuinely consider, and it's nearly free: build.mjs already computes a SHA-256 over the renderer bundle to derive the build identity, so the same machinery could emit a Subresource Integrity attribute on the renderer <script>. That would move "an export cannot silently use a different build" from an application-level check the renderer performs on itself to something the browser enforces before executing a byte. Details in Stage 2.

Risk — no Stage 1e high-risk path match. The elevated risk here isn't in the file list, it's that this base branch gets no CI at all, which I've treated as the central finding of the review.

Moving on to code review. 🔍

中文说明

感谢贡献!问题是真实存在且量化得很清楚的,gate 通过。进入代码审查前有几点需要说明。

模板 —— 内容实质完整,但有两处标题与 .github/pull_request_template.md 不一致:## Why 应为 ## Why it's needed### Evidence 应为 ### Evidence (Before & After),且没有独立的 ### Tested on 标题(OS/状态表格是有的,只是嵌在 Evidence 下面)。审查者需要的信息都齐了,所以不因措辞卡住——下次修改正文时对齐一下即可。

问题 —— 已观测到的缺陷,不是理论性加固。#11031 给出了字节数(19,526,066 raw / 4,772,314 gzip,对比早期导出的 311,990 bytes,约 62.6 倍)、真实单轮 TUI 文件 19,527,461 bytes,以及四步复现。渲染器属于应用代码,却被复制进每一个会话产物;这是明确的缺陷。issue 同时要求增加自动化体积预算,本 PR 也做到了。

方向 —— 与关联 P1 issue 明确期望的方案一致(只保留会话数据和一小段启动代码,渲染器与 React 从不可变的精确版本 URL 加载)。但我仍然选择标记而不是直接放行,原因有两点:

  • 它触及发布打包prepare-package.js 的发布门禁、npm files 列表、standalone 压缩包排除项),并把导出文档的运行时契约从自包含改成了依赖网络。这些正是我应当上报而非自行决定的敏感区域。
  • 它有意放宽了一条已成文的安全不变量chat-transcript-contract-prevalidation.md §10.9 原标题为「CSP 与零网络」,要求打开本地 HTML 不产生未登记 subrequest;本 PR 将其改为「CSP 与登记网络」,并重写条款以允许版本绑定的 renderer 与 React runtime。在同一个 PR 里同步更新文档是正确做法——只是导出文件包含完整会话内容,这个决定应当由人来负责。

本 PR 还叠加在仍未合并的 #9812 之上,base 是 codex/remove-webui 而非 main。因此还不能作为一个可合并单元来判断;而且(见 Stage 2)正是这个 base 选择导致 CI 完全没有运行。

规模 —— 未触及 Stage 0 核心路径,两层核心门禁不适用(且作为 maintainer 提交的 PR 本身也是豁免的)。供参考:非测试代码 141 行(构建/打包 122 行 + 文档 19 行),测试 153 行,生成/schema 0 行,共 12 个文件。远低于所有阈值。

方案 —— 范围合理,没有需要砍掉的部分;diff 严格限制在导出构建、模板、打包链路及其对应测试内,没有夹带无关重构。有一点确实值得考虑,而且几乎零成本:build.mjs 已经为计算构建标识对渲染器 bundle 做过一次 SHA-256,同一套机制完全可以顺带在渲染器 <script> 上输出 Subresource Integrity 属性。这样「导出不会静默使用不同构建」就从渲染器对自身做的一次应用层检查,变成了浏览器在执行任何一个字节之前就强制完成的校验。详见 Stage 2。

风险 —— Stage 1e 高风险路径无命中。这里真正升级的风险不在文件列表里,而在于这个 base 分支完全不会触发 CI——我把它作为本次审查的核心结论。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 45cbb0ee7bb83607ad39304ca8b4ebcd3d6ee1e7 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

I read the PR description first and wrote down what I'd do before opening the diff: keep the transcript data, CSS and nonce inline, build the renderer once per release, publish it as a package asset, and point the document at an exact-version URL with a content-derived identity so a document can't be paired with the wrong renderer. That's what this does, and the execution is more careful than I expected in several places — so most of what follows is hardening, not defects.

What I verified as correct, because it would have been easy to get wrong:

  • The version source change in build.mjs (from packages/web-templates/package.json to the repo-root package.json) is a genuine fix, not a cosmetic path edit. scripts/prepare-package.js builds the published manifest with name: rootPackageJson.name, version: rootPackageJson.version, so root is the authoritative source for the @qwen-code/qwen-code@<version> URL. Both read 0.23.0 today, so the emitted string doesn't change — but the coupling now points at the file that actually decides the published version.
  • The import map's four entries exactly cover what the entry module imports: react, react-dom (for flushSync), react-dom/client (for createRoot), and react/jsx-runtime from the production JSX transform. No specifier is missing today.
  • The import map <script> precedes the module <script> in document order, so the map is registered before resolution starts.
  • The new load-failure handler targets #app, which exists in the template, and document-main.tsx mounts into that same node — so the two paths can't fight over different containers.
  • The two failure modes are cleanly separated and both fail closed: a renderer that never arrives hits the new element-level error listener ("Unable to load this chat export"), while a renderer that arrives but disagrees on rendererVersion throws inside readTranscriptDocument and is caught by the pre-existing DocumentErrorBoundary ("Unable to open this chat export"). Both set data-render-complete="error", which is what the tests poll on.
  • Dropping the <script-sequence escaping and the refusing to inline guard is correct — the JS bundle is no longer inlined into HTML, so the hazard those lines existed for is gone.

1. Nothing asserts that every externalized specifier has an import-map entry (Suggestion — the one I'd most want addressed)

build.mjs externalizes ['react', 'react/*', 'react-dom', 'react-dom/*'] — wildcards — but documentImportMap hardcodes exactly four keys. Those happen to be the right four today. The problem is that the real specifier set isn't decided by document-main.tsx; it's decided by the entire @qwen-code/web-shell graph bundled underneath it, where every bare react/* or react-dom/* import stays external. If a future Web Shell dependency imports something like react-dom/server or react/jsx-dev-runtime, esbuild leaves it bare, the map has no entry, the browser fails module resolution, and every export shows "Unable to load this chat export" at view time while the build and the whole test suite stay green.

That's the worst shape of failure this PR can have: silent at build time, total at runtime, and invisible to CI. And it's cheap to close — build.mjs already throws on an unreplaced placeholder and on unresolvable React versions, so it has the right instincts. Extract the bare specifiers from documentJsBundle.text after the build and assert each one is a key of documentImportMap.imports. Roughly ten lines, and it converts a view-time failure into a build failure.

2. The renderer loads without Subresource Integrity (Suggestion)

document-index.html loads the renderer with src and a nonce but no integrity attribute. Meanwhile build.mjs already computes createHash('sha256').update(documentJsBundle.contents) to derive the build identity — the hash machinery is sitting right there.

The renderer's own value.rendererVersion !== __EXPORT_TRANSCRIPT_RENDERER_VERSION__ check does catch a wrong build, but it runs inside the fetched code: a CDN serving altered bytes executes first and validates second. An integrity attribute reverses that — the browser refuses the bytes before any of them run. Given that the page's payload is the user's full session transcript, that's the highest-value content in the document.

Two honest caveats. The existing hash is over the pre-substitution bundle, so it can't be reused directly — SRI has to be computed over the final documentJs. And React/ReactDOM come from jsDelivr's /+esm builds, whose bytes aren't stable enough to pin with SRI, so trust in jsDelivr can't be fully eliminated this way. It's partial hardening, which is why I'm calling it a suggestion rather than a blocker — but it's partial hardening that costs almost nothing.

3. CSP allows the entire jsDelivr host rather than the pinned paths (Suggestion)

script-src 'nonce-__EXPORT_NONCE__' https://cdn.jsdelivr.net permits any script from that origin, and jsDelivr serves arbitrary npm packages — so the allowed origin is effectively a general-purpose script host. The nonce exists precisely to stop unexpected scripts; the broad host allowance punches a hole in it.

CSP host-source syntax accepts a path with prefix matching, and all five URLs are already known at build time (they're constructed in build.mjs two lines apart). Narrowing to the renderer path plus the four React paths costs one string built alongside documentImportMap.

To be clear about severity: this is defense-in-depth, not a live hole. The transcript is React-rendered and inline scripts are nonce-gated, so exploiting the broad allowance needs an injection vector that doesn't currently exist in the document.

4. scheduler is not pinned by this repo (Note)

The import map pins react, react-dom, react/jsx-runtime and react-dom/client from package-lock.json (both 19.2.4). scheduler is not in the map — it's resolved from inside jsDelivr's own react-dom/+esm build, so its version is chosen by the CDN, not by this repo's lockfile. Your E2E comment confirms Scheduler loading from a pinned CDN URL, which is consistent with that. Worth one line in docs/design/html-export-cdn-renderer.md, since the design claim is exact-version pinning and this is the one runtime module that isn't pinned here.

5. The release-window 404, stated plainly (Note — already documented by you, flagging the shape of it)

v0.23.0 was published 2026-09-03 and cannot contain export-transcript-document.js, because this PR is what adds the file to copy_bundle_assets.js and to the published files list. The tree still reads 0.23.0, so until a version bump ships, the emitted URL points at an already-published, immutable version that will never contain the asset — every export fails closed.

Both the PR body and the design doc say this, so it's a conscious tradeoff rather than an oversight, and I'm not asking for a network preflight at export time (that would be its own problem). What I'd flag is where the failure surfaces: the person running /export html gets a successful-looking 7 KB file, and the person who opens it later gets "Check your network connection and Qwen Code version" — a message that points at the network when the actual cause is a version that predates the asset. If the wording is cheap to adjust, that message is where a confused user will land.

I did not probe the CDN; this is inferred from the release date and from the asset being introduced here.

6. Unit coverage moved, and the destination never executed (Note)

html.test.ts drops the data-document-metadata, Context Usage and expand/collapse/theme-toggle assertions. That's a necessary consequence — those strings lived in the inlined renderer bundle — and the coverage does exist in the Playwright browser gate. But see the CI section below: the browser gate has not run on this PR.

The new expect(html.length).toBeLessThan(500_000) is the automated budget #11031 asked for. It pins template overhead on a small fixture, which is enough to catch the renderer being re-inlined (19.5 MB would blow through it). It does not bound real session payloads, which is fine — that's what EXPORT_TRANSCRIPT_MAX_ENVELOPE_BYTES is for.

7. The browser gate never exercises the import map (Note — deliberate, but worth stating)

integration-tests/chat-transcript-document.test.ts re-bundles the published renderer asset with buildSync({ bundle: true, … }) and no external key, so React resolves from node_modules and gets inlined into the test asset. The route then serves that asset for RENDERER_URL, and the test asserts allowedScriptRequests equals exactly [RENDERER_URL] — which proves no React fetch occurred.

This is deliberate and documented (docs/design/html-export-cdn-renderer.md: "the browser gate fulfills it with a locally bundled React runtime so CI remains independent of CDN availability"), and keeping CI off the network is the right call — I'd make the same choice. The consequence is that the import map is only ever asserted as a string, by the regexes in html.test.ts, and never executed. That's exactly the gap finding 1 would close at build time, which is why I ranked it first.

8. A deleted comment still applies (Nit)

The "Function-form replacers" comment was removed along with the JS-inlining code, but .replace('__DOCUMENT_INLINE_CSS__', () => documentCssBundle.text.trim()) still feeds bundle content through String.prototype.replace, where $&, $` and $' sequences in the CSS would be interpreted as substitution patterns. The hazard the comment described is still live on the CSS path even though the JS path is gone — the comment was doing real work and outlived the code it was deleted with.

Flow as it now stands

sequenceDiagram
    participant P1 as Qwen Code CLI
    participant P2 as Exported HTML file
    participant P3 as jsDelivr CDN
    participant P4 as Browser
    P1->>P2: write transcript data, CSS, nonce, import map and renderer URL
    Note over P1,P2: rendererVersion stamped as package version plus 16 hex build id
    P4->>P2: open the exported file
    P4->>P3: fetch renderer module at the exact package version
    P4->>P3: fetch react and react-dom resolved by the import map
    alt renderer never arrives
        P3-->>P4: error event on the script element
        Note over P4: render-complete error, Unable to load this chat export
    else renderer arrives, build id disagrees
        P4->>P4: readTranscriptDocument throws, error boundary catches
        Note over P4: render-complete error, Unable to open this chat export
    else renderer arrives, build id matches
        P4->>P4: mount transcript into the app node
        Note over P4: render-complete true
    end
Loading
Files changed (12)
File What changed
packages/web-templates/src/export-html/build.mjs The substantive change. Renderer output moves from an IIFE in an outdir to a single ESM asset; React becomes external and is pinned through a generated import map read from the root lockfile; version source moves to the root package.json; a 16-hex content-derived build id is appended to the renderer version and substituted for a placeholder. Loses the inline-script escaping and its guard.
packages/web-templates/src/export-html/src/document-index.html CSP script-src gains the jsDelivr host; the inline renderer script is replaced by an import map plus an external module script; adds a capture-phase error listener that writes the load-failure message into the app node.
scripts/copy_bundle_assets.js Copies the built renderer into dist, warning rather than failing when absent so cli-only dev bundles still work.
scripts/prepare-package.js Adds the renderer to the release artifact gate and to the published files list — this is what makes it reachable on the CDN.
scripts/create-standalone-package.js Adds the renderer to the npm-only exclusion set so standalone archives consume the CDN asset instead of carrying it.
scripts/build.js Comment only, updated to match the new publish-not-inline model.
integration-tests/chat-transcript-document.test.ts Splits the network probe into allowed versus unexpected requests and serves a locally re-bundled renderer for the pinned URL; adds a fail-closed test for an unreachable renderer; gives each invalid-document case its own page.
packages/cli/src/ui/utils/export/formatters/html.test.ts Adds CDN URL, renderer version format, React pinning and a 500 KB size-budget assertion; drops five assertions whose strings lived in the now-external renderer bundle.
scripts/tests/package-assets.test.js Covers the renderer reaching dist and the published files list.
scripts/tests/install-script.test.js Covers the renderer being excluded from standalone archives.
docs/design/html-export-cdn-renderer.md New short design note; states the CDN dependency and that the browser gate fulfills it locally.
docs/design/web-shell/chat-transcript-contract-prevalidation.md Rewrites the zero-network invariant to registered-network in five places, including the section 10.9 heading.

Test evidence

There is no automated test evidence for this PR, and the reason is structural rather than a red suite. .github/workflows/ci.yml triggers pull_request only on branches: ['main', 'release/**']. This PR targets codex/remove-webui, so the entire Qwen Code CI workflow — build, lint, typecheck, unit tests, and the no-AK integration lane — never ran against it.

Of the 20 check-runs on the reviewed commit, exactly one came from a pull_request-event workflow (tui-parity, which triggers on paths). The rest are pull_request_target bot orchestration. verify and tmux-testing are both skipped, not failed.

So, explicitly:

  • not verified: npm run typecheck — never ran on this commit.
  • not verified: npm run lint — never ran on this commit.
  • not verified: packages/cli/src/ui/utils/export/formatters/html.test.ts — never ran on this commit.
  • not verified: scripts/tests/package-assets.test.js, scripts/tests/install-script.test.js — never ran on this commit.
  • not verified: the Playwright browser gate in integration-tests/chat-transcript-document.test.ts, including the new fail-closed case — never ran on this commit. This matters more than the others, because it is now the only place the expand/collapse/theme coverage from finding 6 lives.

The E2E report in your PR comment (7,017-byte export, Chromium rendering with React/ReactDOM/JSX runtime/Scheduler from pinned CDN URLs, blocked-renderer error page, 5/5 browser gate) is your claim from a macOS run, not evidence this review reproduced — I did not build or execute any PR-derived code, per the static-review rule. It's a thorough report and it's the only signal that the real CDN path works end to end, but it is one platform, one run, and unverifiable from here.

The practical consequence: when this is retargeted to main after #9812 merges, CI will run for the first time and any of the above could still move. I'd treat the current state as unreviewed-by-CI rather than as passing.

Check Conclusion
OpenTUI no-flicker gate success
TUI parity snapshots (ink vs opentui) success
ack-review-request skipped
assign success
authorize success
authorize success
delay-automatic-review success
label success
label success
precheck-pr skipped
precheck-pr skipped
publish-resolution skipped
publish-tmux skipped
publish-verify skipped
resolve-pr skipped
review-config skipped
review-pr in_progress
tmux-testing skipped
triage in_progress
verify skipped

Sandboxed verification would settle this: @qwen-code /verify — that the emitted HTML really collapses from ~19.5 MB to kilobytes, that the renderer URL and import map resolve against a real browser, and that a blocked renderer request fails closed rather than blanking, are none of them observable from this diff, and no CI ran against this base to execute the browser gate that covers them. @qwen-code /tmux would cover the other half: driving a real session and /export html through the TUI, which is the path the size claim is actually about. You have write access, so neither needs sponsoring.

中文说明

代码审查

我先读了 PR 描述并写下自己会怎么做,然后才打开 diff:保留会话数据、CSS 和 nonce 内联,渲染器每次发布只构建一次并作为包资产发布,文档指向精确版本 URL,并带上由内容推导出的标识以避免文档与错误的渲染器配对。本 PR 正是这样做的,而且在好几个地方比我预期的更严谨——所以下面大部分是加固建议,不是缺陷。

我核实为正确的部分(这些地方很容易写错):

  • build.mjs 把版本来源从 packages/web-templates/package.json 改到仓库根 package.json,是实质修正而非单纯改路径。scripts/prepare-package.jsname: rootPackageJson.name, version: rootPackageJson.version 生成发布清单,所以根 package.json 才是 @qwen-code/qwen-code@<version> URL 的权威来源。今天两者都是 0.23.0,输出的字符串没变——但耦合现在指向了真正决定发布版本的文件。
  • import map 的四项恰好覆盖入口模块的导入:reactreact-domflushSync)、react-dom/clientcreateRoot),以及 production JSX 转换产生的 react/jsx-runtime。目前没有缺失的 specifier。
  • import map 的 <script> 在文档顺序上先于 module <script>,因此解析开始前映射已注册。
  • 新的加载失败处理指向 #app,模板中确实存在该节点,且 document-main.tsx 挂载到同一节点——两条路径不会争抢不同容器。
  • 两种失败模式分离清晰且都 fail closed:渲染器根本没到达时命中新增的元素级 error 监听("Unable to load this chat export");渲染器到达但 rendererVersion 不一致时在 readTranscriptDocument 内抛出,被既有的 DocumentErrorBoundary 捕获("Unable to open this chat export")。两者都设置 data-render-complete="error",正是测试轮询的字段。
  • 删除 <script 序列转义和 refusing to inline 守卫是正确的——JS bundle 不再内联进 HTML,这些代码存在的风险已经消失。

1. 没有任何检查保证被 external 的 specifier 都在 import map 里(建议——我最希望处理的一条)

build.mjs['react', 'react/*', 'react-dom', 'react-dom/*'] 设为 external——是通配符——但 documentImportMap 只硬编码了四个键。今天这四个恰好是对的。问题在于真正的 specifier 集合不由 document-main.tsx 决定,而由它下面被打包进来的整个 @qwen-code/web-shell 依赖图决定,其中每一个裸 react/*react-dom/* 导入都保持 external。如果将来某个 Web Shell 依赖导入了 react-dom/serverreact/jsx-dev-runtime 之类,esbuild 会保留裸导入,映射表没有对应项,浏览器模块解析失败,于是每一个导出文件在打开时都显示 "Unable to load this chat export",而构建和整套测试全部保持绿色。

这是本 PR 可能出现的最糟糕的失败形态:构建期静默、运行时全面失效、CI 完全看不见。而且修复很便宜——build.mjs 已经会在占位符未替换和 React 版本无法解析时抛错,思路是对的。构建后从 documentJsBundle.text 提取裸 specifier,断言每一个都是 documentImportMap.imports 的键即可。大约十行,就能把打开时的失败变成构建期的失败。

2. 渲染器加载时没有 Subresource Integrity(建议)

document-index.htmlsrc 加 nonce 加载渲染器,但没有 integrity 属性。而 build.mjs 已经在用 createHash('sha256').update(documentJsBundle.contents) 推导构建标识——哈希机制就在那里。

渲染器自身的 value.rendererVersion !== __EXPORT_TRANSCRIPT_RENDERER_VERSION__ 检查确实能发现错误的构建,但它运行在被取回的代码内部:CDN 若返回被篡改的字节,会先执行、后校验。integrity 属性把这个顺序反过来——浏览器在任何字节运行之前就拒绝。考虑到页面载荷是用户完整的会话内容,这是文档中价值最高的数据。

两点如实说明。现有哈希是对替换前的 bundle 计算的,不能直接复用——SRI 必须对最终的 documentJs 计算。而 React/ReactDOM 来自 jsDelivr 的 /+esm 构建,其字节不够稳定,无法用 SRI 固定,所以这条路径无法完全消除对 jsDelivr 的信任。这是部分加固,因此我列为建议而非阻塞项——但它的成本几乎为零。

3. CSP 放行整个 jsDelivr 主机而非固定路径(建议)

script-src 'nonce-__EXPORT_NONCE__' https://cdn.jsdelivr.net 允许该来源下任意脚本,而 jsDelivr 提供任意 npm 包——所以被放行的来源实际上是一个通用脚本托管点。nonce 的存在正是为了阻止意外脚本,宽泛的主机放行在上面开了个口子。

CSP 的 host-source 语法支持带路径的前缀匹配,而且这五个 URL 在构建期全部已知(它们在 build.mjs 中相距两行)。收窄到渲染器路径加四个 React 路径,只需在 documentImportMap 旁边多构造一个字符串。

严重性上说清楚:这是纵深防御,不是当前存在的漏洞。会话内容由 React 渲染、内联脚本受 nonce 保护,所以要利用这条宽泛放行需要一个文档中目前不存在的注入向量。

4. scheduler 不由本仓库固定(说明)

import map 依据 package-lock.json 固定了 reactreact-domreact/jsx-runtimereact-dom/client(均为 19.2.4)。scheduler 不在映射表里——它由 jsDelivr 自己的 react-dom/+esm 构建内部解析,因此版本由 CDN 决定,而非本仓库的 lockfile。你的 E2E 评论确认 Scheduler 从固定 CDN URL 加载,与这一点一致。建议在 docs/design/html-export-cdn-renderer.md 补一句,因为设计主张是精确版本固定,而这是唯一一个不在此处固定的运行时模块。

5. 发布窗口期的 404,直说(说明——你已记录,我标记的是它的形态)

v0.23.0 于 2026-09-03 发布,不可能包含 export-transcript-document.js,因为正是本 PR 才把该文件加进 copy_bundle_assets.js 和发布的 files 列表。代码树仍然是 0.23.0,所以在版本升级发布之前,生成的 URL 指向一个已发布且不可变、永远不会包含该资产的版本——每个导出都会 fail closed。

PR 正文和设计文档都说明了这点,所以这是有意识的取舍而非疏漏,我也不要求在导出时做网络预检(那会带来它自己的问题)。我要标记的是失败出现在哪里:执行 /export html 的人拿到一个看起来正常的 7 KB 文件,而之后打开它的人看到 "Check your network connection and Qwen Code version"——这句提示把原因指向网络,而真正原因是版本早于该资产。如果改措辞成本很低,困惑的用户最终会落在这句话上。

我没有探测 CDN;这是从发布日期和该资产由本 PR 引入这两点推断的。

6. 单测覆盖发生了迁移,而迁移目的地从未执行(说明)

html.test.ts 删掉了 data-document-metadataContext Usage 以及展开/折叠/主题切换的断言。这是必然结果——那些字符串原本存在于内联的渲染器 bundle 中——而覆盖确实存在于 Playwright browser gate 里。但请看下面的 CI 部分:browser gate 在本 PR 上没有运行过。

新增的 expect(html.length).toBeLessThan(500_000) 就是 #11031 要求的自动化预算。它用小 fixture 固定模板开销,足以捕获渲染器被重新内联的情况(19.5 MB 会直接超出)。它不约束真实会话载荷,这没问题——那是 EXPORT_TRANSCRIPT_MAX_ENVELOPE_BYTES 的职责。

7. browser gate 从未执行 import map(说明——有意为之,但值得点明)

integration-tests/chat-transcript-document.test.tsbuildSync({ bundle: true, … })没有 external重新打包已发布的渲染器资产,因此 React 从 node_modules 解析并被内联进测试资产。随后路由为该固定 URL 提供这份资产,测试断言 allowedScriptRequests 恰好等于 [RENDERER_URL]——这证明了没有发生 React 请求。

这是有意设计并已记录(docs/design/html-export-cdn-renderer.md:"the browser gate fulfills it with a locally bundled React runtime so CI remains independent of CDN availability"),让 CI 不依赖网络是正确的选择——我也会这么做。其结果是 import map 只曾被当作字符串断言过(html.test.ts 里的正则),从未被执行。这正是第 1 条建议在构建期能够补上的缺口,也是我把它排在第一位的原因。

8. 一处被删除的注释其实仍然适用(细节)

"Function-form replacers" 注释随 JS 内联代码一起被删除,但 .replace('__DOCUMENT_INLINE_CSS__', () => documentCssBundle.text.trim()) 仍然把 bundle 内容送进 String.prototype.replace,其中 CSS 里的 $&$`$' 序列会被解释为替换模式。该注释描述的风险在 CSS 路径上依然存在,尽管 JS 路径已经消失——这条注释仍在发挥作用,却和它并非一同失效的代码一起被删掉了。

测试证据

本 PR 没有任何自动化测试证据,原因是结构性的,而不是套件变红。 .github/workflows/ci.ymlpull_request 触发仅限 branches: ['main', 'release/**']。本 PR 的 base 是 codex/remove-webui,所以整个 Qwen Code CI workflow——构建、lint、typecheck、单元测试、no-AK 集成通道——从未针对它运行。

被审 commit 上的 20 个 check-run 中,只有一个来自 pull_request 事件的 workflow(tui-parity,按 paths 触发)。其余都是 pull_request_target 的机器人编排作业。verifytmux-testing 都是 skipped,不是失败。

因此明确列出:

  • 未验证: npm run typecheck —— 该 commit 上从未运行。
  • 未验证: npm run lint —— 该 commit 上从未运行。
  • 未验证: packages/cli/src/ui/utils/export/formatters/html.test.ts —— 该 commit 上从未运行。
  • 未验证: scripts/tests/package-assets.test.jsscripts/tests/install-script.test.js —— 该 commit 上从未运行。
  • 未验证: integration-tests/chat-transcript-document.test.ts 中的 Playwright browser gate,包含新增的 fail-closed 用例 —— 该 commit 上从未运行。这一条比其他都重要,因为它现在是第 6 条中展开/折叠/主题覆盖唯一的落脚点。

你 PR 评论里的 E2E 报告(7,017 bytes 导出、Chromium 中 React/ReactDOM/JSX runtime/Scheduler 从固定 CDN URL 加载、渲染器请求被阻断时的错误页、browser gate 5/5)是你在 macOS 上的一次自述结果,不是本次审查复现出的证据——按照静态审查规则,我没有构建或执行任何 PR 派生代码。这份报告很详实,也是真实 CDN 路径端到端可用的唯一信号,但它是一个平台、一次运行,且从这里无法验证。

实际后果是:等 #9812 合并、本 PR 重新指向 main 后,CI 才会第一次运行,上述任何一项都可能发生变化。我会把当前状态视为「未经 CI 审查」,而不是「已通过」。

沙箱化验证可以定论:@qwen-code /verify —— 生成的 HTML 是否真的从约 19.5 MB 收缩到千字节级、渲染器 URL 与 import map 是否能在真实浏览器中解析、渲染器请求被阻断时是否 fail closed 而非白屏,这些都无法从 diff 观察,而针对这个 base 也没有 CI 去执行覆盖它们的 browser gate。@qwen-code /tmux 可以覆盖另一半:在 TUI 中驱动真实会话并执行 /export html,也就是体积主张真正针对的那条路径。你有写权限,两者都不需要他人代为发起。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 45cbb0ee7bb83607ad39304ca8b4ebcd3d6ee1e7 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 3/5 — the engineering is careful and the size win is enormous, but nothing automated has executed against this commit, and one failure mode is invisible to every layer of testing.

I wrote my own approach down before reading the diff, and it came out essentially identical to yours: keep the data, CSS and nonce inline, build the renderer once per release, publish it as a package asset, point the document at an exact-version URL, and bind it with a content-derived identity. So I don't have a simpler design to argue for — which means the real question isn't whether this is the right shape, it's whether the edges are covered. On the shape, I'm convinced. 19.5 MB down to 7 KB for a one-turn session is not a marginal improvement, and the reasoning in #11031 (the renderer is application code, not session data) is the correct diagnosis rather than a workaround.

What I keep coming back to is that the parts I could verify, I verified, and they held up better than I expected. Pointing the version source at the root package.json is quietly the right call, because that's the file prepare-package.js actually builds the published manifest from — the previous source only happened to agree. Splitting "renderer never arrived" from "renderer arrived and disagrees" into two distinct messages via two distinct mechanisms is the kind of thing people usually collapse into one blank page. And rewriting the design doc's zero-network invariant in the same PR, rather than letting it silently go stale, is exactly right.

But I can't get past the fact that no automated check has run against this commit. Not typecheck, not lint, not the unit tests, not the Playwright browser gate — because ci.yml only triggers on main and release/** and this targets codex/remove-webui. Approving would mean signing off on a twelve-file change to release packaging and browser runtime loading on the strength of a static read plus one macOS report I can't reproduce. That's not a judgement I should make.

The specific thing that worries me is finding 1, and it worries me because of its shape rather than its likelihood. The import map is four hardcoded keys against a wildcard external pattern, and the actual specifier set is decided by the whole Web Shell dependency graph underneath. Today they match — I checked. But if they ever stop matching, the build stays green, the unit tests stay green, the browser gate stays green (it re-bundles the renderer with React inlined, so it structurally cannot exercise the import map), and the only place the failure appears is a user opening an exported file and being told to check their network connection. Every layer of verification this repo has is blind to it. Ten lines of build-time assertion would change that, and the file already throws for two less likely conditions.

On the security side I'm deliberately not making the call. Turning a self-contained document into one that executes remote code is a real change to a documented invariant, and doing it without Subresource Integrity — when the build already computes a SHA-256 over the very bytes in question — is a tradeoff a maintainer should own rather than a bot. I've laid out what SRI would and wouldn't buy (it can't cover jsDelivr's /+esm React builds) in Stage 2; that's the honest version, not a clean win either way.

Six months from now I'd thank you for the size fix and for updating the design doc straight, and I'd curse the missing import-map assertion the first time a Web Shell dependency reaches for a React subpath. That's a 3, not a 4.

What I'd want before approving: CI that actually runs — which realistically means once #9812 merges and this is retargeted to main; the import-map coverage assertion; and a maintainer's explicit yes-or-no on the SRI and CSP-narrowing tradeoffs rather than my inference about them.

I'm not requesting changes, because I found no defect I can prove — the concerns above are hardening and a missing CI signal, not a broken diff. And I'm not approving, because "the direction looks correct" isn't the standard.

⏸️ Deferring — and note that this defer is unaddressed. The deterministic resolver found nobody to hand it to: this PR carries no labels, the owner map's nine areas are all core* keyed on category/core and scope/core, so even the labels on #11031 (scope/web-shell, scope/build-system, roadmap/export-data) wouldn't match an area; there's no prior human review to fall back to; and QWEN_MAINTAINER_HANDLE isn't set. I won't guess a login, so no one gets notified by this comment. A maintainer reading the thread needs to pick it up — and separately, the owner map having no coverage for web-shell, export, or packaging is worth fixing on its own, since this is the second mechanism in this run that produced nothing.

中文说明

Confidence: 3/5 —— 工程实现很细致,体积收益巨大,但针对这个 commit 没有任何自动化检查执行过,而且有一种失败形态对所有测试层都是不可见的。

我在打开 diff 之前先写下了自己的方案,结果和你的基本一致:数据、CSS 和 nonce 保持内联,渲染器每次发布构建一次,作为包资产发布,文档指向精确版本 URL,并用由内容推导的标识绑定。所以我没有更简单的设计可主张——这意味着真正的问题不是形态对不对,而是边角有没有覆盖。形态上我是信服的。单轮会话从 19.5 MB 降到 7 KB 不是边际改进,而 #11031 的推理(渲染器是应用代码,不是会话数据)是正确的诊断,不是绕过。

我反复回想的是:我能核实的部分我都核实了,而且它们比我预期的更站得住。把版本来源指向根 package.json 是低调但正确的选择,因为那才是 prepare-package.js 实际用来生成发布清单的文件——之前的来源只是碰巧一致。把「渲染器根本没到达」和「渲染器到达但不一致」用两套不同机制拆成两条不同提示,是通常会被人们合并成一个白屏的处理。而在同一个 PR 里重写设计文档的零网络不变量、而不是让它悄悄失效,完全正确。

但我过不去的一点是:针对这个 commit 没有任何自动化检查运行过。typecheck 没有、lint 没有、单元测试没有、Playwright browser gate 也没有——因为 ci.yml 只在 mainrelease/** 上触发,而本 PR 的 base 是 codex/remove-webui。批准就意味着仅凭一次静态阅读加一份我无法复现的 macOS 报告,去签发一个涉及发布打包和浏览器运行时加载的十二文件改动。这个判断不该由我来做。

具体让我担心的是第 1 条,而且我担心的是它的形态而非概率。import map 是四个硬编码键对应一个通配符 external 模式,而真实的 specifier 集合由其下整个 Web Shell 依赖图决定。今天两者是匹配的——我核对过。但一旦不再匹配,构建保持绿色、单元测试保持绿色、browser gate 也保持绿色(它把 React 内联后重新打包渲染器,因此结构上就无法执行 import map),唯一暴露问题的地方是用户打开导出文件、然后被告知去检查网络连接。这个仓库的每一层验证对它都是盲的。十行构建期断言就能改变这一点,而这个文件已经为两个更不可能发生的条件抛错了。

安全方面我是刻意不做决定的。把自包含文档变成执行远程代码的文档,是对一条成文不变量的实质改变;而在构建过程已经对同一批字节计算过 SHA-256 的情况下不带 Subresource Integrity,这个取舍应当由 maintainer 负责,而不是机器人。我在 Stage 2 里说明了 SRI 能买到什么、买不到什么(它覆盖不了 jsDelivr 的 /+esm React 构建);那是如实的版本,两边都不是干净的胜利。

六个月后我会感谢你的体积修复和对设计文档的直接更新,也会在某个 Web Shell 依赖第一次伸手去拿 React 子路径时咒骂缺失的 import-map 断言。这是 3 分,不是 4 分。

批准之前我希望看到: CI 真正运行起来——现实地说就是等 #9812 合并、本 PR 重新指向 main 之后;import-map 覆盖断言;以及 maintainer 对 SRI 与 CSP 收窄这两项取舍明确的同意或否决,而不是我对它们的推断。

我不会 request changes,因为我没找到能证明的缺陷——上面的顾虑是加固和缺失的 CI 信号,不是一个坏掉的 diff。我也不会 approve,因为「方向看起来对」不是标准。

⏸️ 转交人工——并且请注意这次转交没有落到任何人身上。 确定性解析器没有找到可交接的对象:本 PR 没有任何标签,owner map 的九个 area 全部是 core*、以 category/corescope/core 为键,所以即使 #11031 上的标签(scope/web-shellscope/build-systemroadmap/export-data)也匹配不到任何 area;没有既有的人工 review 可回退;QWEN_MAINTAINER_HANDLE 也未设置。我不会去猜一个 login,因此这条评论不会通知到任何人。需要读到这个线程的 maintainer 主动接手——另外,owner map 对 web-shell、export 和 packaging 完全没有覆盖,这件事本身就值得修,因为这已经是本次运行中第二个什么都没产出的机制了。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 45cbb0ee7bb83607ad39304ca8b4ebcd3d6ee1e7 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment thread packages/web-templates/src/export-html/src/document-index.html Outdated
Comment thread integration-tests/chat-transcript-document.test.ts Outdated
Comment thread packages/web-templates/src/export-html/src/document-index.html Outdated
Comment thread packages/web-templates/src/export-html/build.mjs Outdated
Comment thread packages/cli/src/ui/utils/export/formatters/html.test.ts
Comment thread integration-tests/chat-transcript-document.test.ts Outdated
Comment thread packages/web-templates/src/export-html/src/document-index.html
Comment thread integration-tests/chat-transcript-document.test.ts Outdated
Comment thread scripts/tests/install-script.test.js
Comment thread scripts/tests/package-assets.test.js
@yiliang114 yiliang114 changed the title fix(export): load transcript renderer from CDN fix(export): host transcript renderer on project OSS Sep 5, 2026
@yiliang114

yiliang114 commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Updated the distribution model in 216b42b78ecb: the export now loads one fully bundled classic renderer from the exact @qwen-code/qwen-code version on unpkg. React and Web Shell remain outside the exported HTML, and there are no import maps or additional runtime requests.

The OSS release/sync changes have been removed. The renderer ships only in the existing npm package, carries SHA-384 SRI over its final bytes, and stays excluded from standalone archives. Full build, typecheck, lint, package tests, npm pack inspection, formatter tests, and the Chromium browser gate pass locally. The only remaining delivery check is a real unpkg smoke test after the first release containing this change.

@yiliang114 yiliang114 changed the title fix(export): host transcript renderer on project OSS fix(export): load transcript renderer from unpkg Sep 5, 2026
@qwen-code-review-bot

Copy link
Copy Markdown
Collaborator

The PR head is unchanged (216b42b7) — the same commit I just verified end to end — so the review stands as completed.

Review of #11035 (verified against head 216b42b7, merge-base 029b41bc):

The implementation is sound and I verified it end to end locally. The one item to fix before merge is that the PR title/body still describe the abandoned OSS design — the code ships npm + unpkg, which is exactly what #11031 asks for.

Verified (local build + tests at the reviewed head):

  • Full --cli-only build chain green. The generated template embeds https://unpkg.com/@qwen-code/qwen-code@0.23.0/export-transcript-document.js with a sha384 integrity that matches the built renderer byte-for-byte; template is 6.6 KB with only __EXPORT_NONCE__ left for export time.
  • Version pinning is consistent across the chain: root package.json version → npm dist package name/version (prepare-package.js) → exact-version unpkg URL; the content-derived +buildId identity is enforced on both the CLI side (formatters/html.ts) and the renderer side (document-main.tsx).
  • Export unit tests: 107 passed. Packaging/installer tests (renderer copied into the npm package, excluded from standalone archives, release gate in verifyBundleArtifacts): 150 passed.
  • All 5 Chromium browser-gate tests passed, including the fail-closed arms: renderer unavailable and SRI integrity mismatch both land on the visible load-error page via real Chromium integrity enforcement; CSP stays connect-src 'none' with zero unregistered requests.
  • Typecheck (cli + integration), eslint on changed files, and check:no-webui all clean. git merge-tree against the current base tip (which moved 2 commits ahead during review: the check:no-webui tightening + docs fixes) merges cleanly with no overlap.
  • Note: ci.yml's pull_request trigger only covers main/release/**, so this stacked PR gets no test CI until it's retargeted — the local run above is the gate in the meantime.

Blocking (metadata, not code): the title says "host transcript renderer on project OSS", and the body describes that abandoned architecture throughout — "loaded from the project's existing Aliyun OSS bucket", the release-asset + OSS-sync byte-for-byte verification flow (no workflow changes exist in this diff), "The renderer is not added to the npm package" (it now is, deliberately), "avoids depending on a third-party JavaScript delivery service" (unpkg is one), and reviewer test-plan step 3 tells reviewers to confirm the OSS URL. The design doc (§10.9) was updated to the unpkg model and the code matches #11031's requested distribution model, so only the PR description is stale — but that description becomes the squash-commit subject and release-notes entry, so it needs rewriting to the npm/unpkg architecture before merge. The body's closing "smoke test after first release" note is still valid, just re-aim it at the unpkg URL.

Non-blocking: opening an export now hard-requires unpkg.com reachability. Every other distribution surface here (install scripts, live-host, desktop updater) deliberately sits on Aliyun OSS, and unpkg has historically been unreliable for mainland-China users — #11031 explicitly chose unpkg, so I'm treating this as a settled decision, but worth having the availability trade-off consciously on record: integrity is covered by SRI, the residual exposure is availability, with a clear fail-closed error.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

@yiliang114
yiliang114 merged commit 64a4373 into codex/remove-webui Sep 5, 2026
15 of 16 checks passed

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ This run could not certify that any of this diff was reviewed. Suggestions are inline.

Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at /home/github-runner/actions-runner-hk1-14/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-14--work-qwen-code-qwen-code/subagents/30d9fba9-7a16-4cc6-977a-057d785dee5e (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-14/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-14--work-qwen-code-qwen-code/subagents/30d9fba9-7a16-4cc6-977a-057d785dee5e'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.

Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at /home/github-runner/actions-runner-hk1-14/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-14--work-qwen-code-qwen-code/subagents/30d9fba9-7a16-4cc6-977a-057d785dee5e (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-14/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-14--work-qwen-code-qwen-code/subagents/30d9fba9-7a16-4cc6-977a-057d785dee5e'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • integration-tests/chat-transcript-document.test.ts:557 — [review] maximum-document gate pins the renderer fetch with arrayContaining while both siblings pin exactly once (code-age: unchanged since the previous round's reviewed head 45cbb0ee…
  • packages/web-templates/src/export-html/build.mjs:22 — [review] renderer-URL/published-version invariant is an unguarded four-level '..' manifest read with no name assertion and no detecting test (code-age: unchanged since the previous round…
中文说明

⚠️ 本次运行无法证明这个 diff 的任何部分经过了审查。 建议见行内评论。

未审查:覆盖情况——无法读取 agent 的运行记录(no subagent transcripts at /home/github-runner/actions-runner-hk1-14/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-14--work-qwen-code-qwen-code/subagents/30d9fba9-7a16-4cc6-977a-057d785dee5e (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-14/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-14--work-qwen-code-qwen-code/subagents/30d9fba9-7a16-4cc6-977a-057d785dee5e'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.),本次运行无法证明 diff 的任何部分被读过。

未审查:验证——无法检查步骤 4 与步骤 5 是否运行(no subagent transcripts at /home/github-runner/actions-runner-hk1-14/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-14--work-qwen-code-qwen-code/subagents/30d9fba9-7a16-4cc6-977a-057d785dee5e (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-14/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-14--work-qwen-code-qwen-code/subagents/30d9fba9-7a16-4cc6-977a-057d785dee5e'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.)。

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +644 to +645
await page.route('**/*', async (route) => {
if (rendererBody && route.request().url() === RENDERER_URL) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-1: This new fail-closed gate hand-rolls its own route handler instead of installing installNetworkAndCspProbe, so it records neither unexpectedRequests nor cspErrors and asserts neither — the design's registered-network contract (§10.9: opening the exported HTML must produce no other unregistered subrequest, and browser tests must fail immediately on any unregistered request or CSP violation) is ungated on exactly the load-error path this test exists to cover. If a future change to the template's error path — a fallback-CDN retry or preconnect, an error-page image or font fetch, a nonce-less inline handler — issues an unregistered subrequest or CSP violation only when the renderer fails to load, this handler silently aborts the request (route.abort('blockedbyclient') does not fail the test) and no CSP error is collected, so the gate stays green while opened exports violate the contract. The three sibling browser gates all assert these channels against the fixture; this one asserts none.

Witness:

witness: not run — the probe was written in a scratch tree (the fail-closed route handler verbatim + assertions over real rendered HTML with an injected unregistered error-path subrequest, plus a flipped gate asserting the fixture's zeros) and the renderer asset built SRI-identical, but chromium.launch() failed with ENOSPC (disk 100% full) and an external pipeline sweep removed the trees before a retry.

Suggested fix: give installNetworkAndCspProbe an opt-in renderer-response mode (a rendererBody parameter; null aborts the renderer URL instead of fulfilling it), install it per iteration here, and after the alert assertion add expect(probe.unexpectedRequests).toHaveLength(expectedNetwork.unexpectedRequests) and expect(probe.cspErrors, probe.cspErrors.join('\n')).toHaveLength(expectedNetwork.cspViolations). The failure mode must be opt-in: the probe currently fulfills url === RENDERER_URL unconditionally (lines 291-300) and three other gates depend on that fulfill (the allowedScriptRequests assertions at lines 613 and 737). Fix witness: with a probe request deliberately injected into the error path, the new unexpectedRequests assertion in this same test must go red.

中文说明

这个新的 fail-closed 门径自己手写 route handler,而没有安装 installNetworkAndCspProbe,因此既不记录也不断言 unexpectedRequests / cspErrors —— 设计文档 §10.9 的登记网络契约(打开导出 HTML 后不得产生其他未登记 subrequest;浏览器测试对任何未登记请求或 CSP violation 必须立即失败)恰恰在这个测试本应覆盖的加载失败路径上没有门禁。如果未来模板错误路径的改动(fallback CDN 重试/预连接、错误页图片或字体请求、无 nonce 的内联 handler)只在 renderer 加载失败时发出未登记 subrequest 或触发 CSP violation,这个 handler 会静默 abort 该请求(route.abort('blockedbyclient') 不会让测试失败),也不会收集 CSP 错误,于是导出文件已经违反契约而门禁依然全绿。其余三个浏览器门径都对照 fixture 断言这些通道,只有这个一个都没断言。

建议修复:给 installNetworkAndCspProbe 增加可选的 renderer 响应模式(rendererBody 参数;为 null 时 abort renderer URL 而不是 fulfill),在本测试中按迭代安装,并在 alert 断言后补充 expect(probe.unexpectedRequests).toHaveLength(expectedNetwork.unexpectedRequests)expect(probe.cspErrors, probe.cspErrors.join('\n')).toHaveLength(expectedNetwork.cspViolations)。失败模式必须是可选启用的:probe 目前无条件 fulfill url === RENDERER_URL(291-300 行),另外三个门径依赖该行为(613 与 737 行的 allowedScriptRequests 断言)。修复验收:向错误路径故意注入一个 probe 请求后,本测试新增的 unexpectedRequests 断言必须变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

},
true,
);
window.addEventListener('unhandledrejection', showLoadError);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-2: The unhandledrejectionshowLoadError wiring is the only guard covering a renderer that loads (SRI passes, no evaluation-time throw) but then fails asynchronously during boot — and it is exercised by no test; nothing in the suite ever produces a promise rejection inside the exported page. Both arms of the new fail-closed test reach showLoadError through the script-element error branch (aborted request; SRI-blocked body), so a future edit that drops this listener or narrows the event.target === window branch leaves an async-boot rejection showing a blank, inert page with no error screen and no data-render-complete='error', and every existing test stays green.

Witness:

four-arm Chromium probe (scratch tree at 216b42b78e, renderer served through the production template's real SRI hash):
A-intact-async-reject     {"dataRenderComplete":"error","appRole":"alert","appEmpty":false}
B-mutant-async-reject     {"dataRenderComplete":null,"appEmpty":true}   ← listener removed: blank page
C-mutant-aborted-request  {"dataRenderComplete":"error"}                ← existing arm, still green on mutant
D-mutant-tampered-body    {"dataRenderComplete":"error"}                ← existing arm, still green on mutant

Suggested fix: add a browser-gate case that injects an inline nonce-bearing script doing Promise.reject(new Error('async boot probe')) before render completes, and asserts data-render-complete='error' plus the alert. Fix witness: that new case must go red when this window.addEventListener('unhandledrejection', showLoadError) registration is removed — the probe above shows nothing goes red for that mutation today.

中文说明

unhandledrejectionshowLoadError 这条链路是唯一覆盖“renderer 加载成功(SRI 通过、求值未抛错)但在异步启动阶段失败”的防线,而它没有任何测试覆盖:整个测试套件从未在导出页面内制造过 promise rejection。新 fail-closed 测试的两个分支都是经由 script 元素级 error 分支到达 showLoadError(请求被 abort;body 被 SRI 拦截),因此未来若删除这个 listener 或收窄 event.target === window 分支,异步启动 rejection 会让页面空白且无任何错误提示(data-render-complete 不会变成 'error'),而所有现有测试仍然全绿。

建议修复:新增一个浏览器门径用例,在渲染完成前注入一个带 nonce 的内联脚本执行 Promise.reject(new Error('async boot probe')),断言 data-render-complete='error' 以及 alert 文案。修复验收:删除这行 window.addEventListener('unhandledrejection', showLoadError) 后,新用例必须变红 —— 上面的探针表明今天没有任何测试会因此变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +75 to +77
const documentRendererIntegrity = `sha384-${createHash('sha384')
.update(documentJs)
.digest('base64')}`;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-3: The load-bearing invariant “template integrity attribute == SHA-384 of the bytes actually published as export-transcript-document.js” has no content-level check on any Node-side lane: no test re-hashes the built asset and compares it to the attribute, every packaging test treats the renderer as an opaque fixture (window.QwenExportRenderer = true;), verifyBundleArtifacts checks existence only, and copy_bundle_assets.js copies byte-for-byte with no hash check. The invariant is pinned solely by real-Chromium SRI inside the browser gate — a suite no workspace test command collects, running only in two dedicated CI legs. A build.mjs regression that hashes different bytes than it writes/publishes (e.g. hashing the pre-placeholder-replacement documentJsBundle.text, which sits ten lines above and differs by exactly the build-id substitution, or a later step rewriting the copied asset) ships exports whose renderer is SRI-blocked for 100% of users — every export shows “Unable to load this chat export” — with every workspace suite, packaging test, and the release gate green.

Witness:

sweep: 0 of 103 createHash/sha384 hits in test files re-hash the asset vs the attribute
       (only integrity assertion anywhere: shape regex, formatters/html.test.ts:66)
lanes: test:ci = workspaces + scripts/tests; browser gate only ci.yml:1421 / e2e.yml:629
probe: E1-integrity-match    {"rendererExecuted":true}
       E2-integrity-mismatch {"rendererExecuted":false,"dataRenderComplete":"error","sriConsoleViolation":true}
       → a hash/asset divergence is caught only because Chromium enforces SRI on route-fulfilled bodies in that one suite

Suggested fix: add a Node-side assertion (beside html.test.ts or a web-templates test) that recomputes sha384 over packages/web-templates/src/export-html/dist/export-transcript-document.js and asserts equality with the integrity="sha384-…" value extracted from the rendered HTML / EXPORT_TRANSCRIPT_HTML_TEMPLATE. The check must hash exactly the bytes the publish pipeline ships: copy_bundle_assets.js copies that dist file byte-for-byte into dist/, and build.mjs writes the same documentJs string (utf8) it hashes — hashing any other file (e.g. the pre-replacement esbuild output) pins the wrong artifact. Fix witness: that new test must go red if .update(documentJs) is changed to .update(documentJsBundle.text) — the mutation changes the hash while every current suite stays green.

中文说明

“模板 integrity 属性 == 实际发布为 export-transcript-document.js 的字节的 SHA-384”这一承重不变量在任何 Node 侧通道上都没有内容级校验:没有测试对构建产物重新计算哈希并与属性比对;所有打包测试把 renderer 当作不透明 fixture(window.QwenExportRenderer = true;);verifyBundleArtifacts 只检查文件存在;copy_bundle_assets.js 只做逐字节复制、不做哈希校验。该不变量唯一的钉住来自浏览器门径里的真实 Chromium SRI —— 而这个套件不被任何 workspace 测试命令收集,只在两个专用 CI leg 中运行。如果 build.mjs 回归为对不同于写入/发布的字节计算哈希(例如改为对占位符替换前的 documentJsBundle.text 计算 —— 它就在上方十行、恰好相差 build-id 替换;或后续步骤改写了复制的资产),发布的每个导出都会被 SRI 拦截 —— 所有导出显示 “Unable to load this chat export” —— 而所有 workspace 套件、打包测试和发布门禁全部保持绿色。

建议修复:新增 Node 侧断言(放在 html.test.ts 旁或 web-templates 测试中),对 packages/web-templates/src/export-html/dist/export-transcript-document.js 重新计算 sha384,并断言与渲染后 HTML / EXPORT_TRANSCRIPT_HTML_TEMPLATE 中提取的 integrity="sha384-…" 相等。校验必须哈希发布流水线实际发布的字节:copy_bundle_assets.js 将该 dist 文件逐字节复制进 dist/,build.mjs 写入的正是它哈希的同一 documentJs 字符串(utf8)—— 哈希任何其他文件(如替换前的 esbuild 输出)都会钉错产物。修复验收:把 .update(documentJs) 改成 .update(documentJsBundle.text) 后,新测试必须变红 —— 该突变会改变哈希,而当前所有套件都保持绿色。

— qwen3.8-max via Qwen Code /review (v0.23.0)

);

browser = await chromium.launch({ headless: true });
for (const rendererBody of [null, 'throw new Error("broken renderer");']) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-4: The “fails integrity” arm of this test cannot actually prove SRI enforcement: the tampered body fails the baked-in sha384 and Chromium blocks it pre-execution (probe-verified), firing the same element-level error branch as the first (null/abort) iteration. The window-level branch — a renderer that passes SRI but throws during evaluation — is therefore pinned by zero tests, and this arm would pass identically if the integrity attribute were ignored or dropped: the body would execute, throw, and hit the event.target === window branch of the same handler, producing the same data-render-complete='error' state and the same alert text.

Witness:

INTACT arm-B (current iteration 2): {"rendererExecuted":false,"sriConsoleViolation":true,"renderComplete":"error"}
MUTANT (removed `event.target === window ||`, rebuilt web-templates):
  real test 'fails closed …'                    → Test Files 1 passed (1)   ← suite green, window branch gone
  arm-C probe (integrity rewritten to broken body's sha384) → AssertionError: expected null to be 'error'  ← blank page
INTACT arm-C: {"rendererExecuted":true,"renderComplete":"error"}   ← fix shape reaches the window branch

Suggested fix: in that iteration (or a sibling), compute sha384-<base64 of the broken body> in the test and rewrite the rendered HTML's integrity attribute before page.setContent, so the broken script passes integrity, executes, throws, and must reach showLoadError() through the event.target === window branch; optionally serve a non-throwing corrupt body (window.__rendererCompromised = true;) and assert the flag stays undefined to pin SRI non-execution directly. Keep issuing exactly one registered renderer request and keep the document's rendererVersion matching EXPORT_TRANSCRIPT_RENDERER_VERSION, or the page takes the DocumentError (“Unable to open”) path instead and the test passes for the wrong reason (document-main.tsx:72). Fix witness: the extended iteration must go red if the event.target === window condition in document-index.html is removed — the probe shows today's suite stays green on that mutant — and the non-execution flag must go red if the integrity attribute is dropped.

中文说明

这个测试的 “fails integrity” 分支实际上无法证明 SRI 生效:被篡改的 body 与内嵌 sha384 不匹配,Chromium 在执行前就将其拦截(已由探针证实),触发的是与第一个(null/abort)迭代完全相同的元素级 error 分支。因此“renderer 通过 SRI 但在求值阶段抛错”的 window 级分支没有任何测试钉住;而且如果 integrity 属性被忽略或删除,这个分支的行为完全不变:body 会执行、抛错、命中同一 handler 的 event.target === window 分支,产生同样的 data-render-complete='error' 状态和同样的 alert 文案。

建议修复:在该迭代(或新增迭代)中由测试计算 sha384-<损坏 body 的 base64>,并在 page.setContent 前重写渲染后 HTML 的 integrity 属性,使损坏脚本通过完整性校验、真正执行并抛错,从而必须经由 event.target === window 分支到达 showLoadError();也可以额外提供一个不抛错的损坏 body(window.__rendererCompromised = true;),断言该标志保持 undefined,直接钉住 SRI 的“不执行”语义。注意保持只发出一次登记的 renderer 请求,且文档的 rendererVersionEXPORT_TRANSCRIPT_RENDERER_VERSION 匹配,否则页面会走 DocumentError(“Unable to open”)路径,测试将因错误原因通过(document-main.tsx:72)。修复验收:删除 document-index.html 中的 event.target === window 条件后,扩展的迭代必须变红 —— 探针表明现有套件对该突变保持绿色;删除 integrity 属性后,非执行标志断言必须变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +34 to +35
app.textContent =
'Unable to load this chat export. This Qwen Code build may not have a published renderer yet; also check your network connection.';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-5: The event.target === window error branch and the unhandledrejection branch route post-load renderer crashes to the same argument-less showLoadError() as CDN/SRI load failures, discarding event.message/event.reason: a renderer that passes SRI, downloads, and then throws during startup tells every user “This Qwen Code build may not have a published renderer yet; also check your network connection” while the renderer is published and the network is fine, and the true error exists only in DevTools — sending oncall after unpkg/npm instead of the renderer stack. In addition, because data-render-complete is set only one requestAnimationFrame after mount (document-main.tsx:236-241), a rejection landing between React commit and that frame — e.g. in a child effect, which DocumentErrorBoundary cannot catch (React boundaries do not catch rejections) — passes the guard and app.textContent = … wipes already-committed content.

Witness:

M1 (renderer startup throw, build re-hashed so SRI passes), INTACT:
  download OK; pageErrors ["Error: renderer startup regression v9probe"]
  alert "Unable to load this chat export. … check your network connection."; realErrorVisibleOnPage: false
M2 flip (showLoadError(detail) passing event.message on the window branch):
  alert "Unable to display this chat export. The renderer failed after it loaded: Uncaught Error: renderer startup regression v9probe"
  realErrorVisibleOnPage: true; cdn-fail arm keeps the "Unable to load…" substring
Wipe (frozen arm, declared rAF stub): BEFORE {renderComplete:null, appChildCount:1, appHtmlLen:2184}
  → rejection → AFTER {renderComplete:"error", appChildCount:0, appHtmlLen:128}; real-window rate 0/40 on this machine

Suggested fix: give showLoadError(detail) a reason argument — from the window-error branch pass event.message and word it as a renderer crash (“Unable to display this chat export. The renderer failed after it loaded: …”); from unhandledrejection pass event.reason; keep the current CDN/publish wording only for the script-element branch (event.target.id === 'transcript-renderer'). Optionally skip the DOM wipe when #app already has children. The load-failure path must keep the substring 'Unable to load this chat export': chat-transcript-document.test.ts:662 asserts it for the CDN-failure class (the probe confirmed the M2 fix shape is compatible). Fix witness: a browser-gate case serving an executing-and-throwing body whose sha384 the test substitutes into the integrity attribute must assert the alert text differs from the CDN-missing wording and names the thrown message; removing the detail plumbing turns it red (probe arms M1/M2 demonstrate the flip).

中文说明

event.target === window 错误分支和 unhandledrejection 分支把“renderer 加载成功之后的崩溃”与 CDN/SRI 加载失败 routed 到同一个不带参数的 showLoadError(),丢弃了 event.message/event.reason:一个通过 SRI、成功下载、却在启动阶段抛错的 renderer 会让所有用户看到 “This Qwen Code build may not have a published renderer yet; also check your network connection”——而 renderer 已发布、网络也正常,真正的错误只存在于 DevTools 中,把排查方向引向 unpkg/npm 而不是 renderer 代码栈。此外,由于 data-render-complete 只在挂载后的一帧 requestAnimationFrame 中设置(document-main.tsx:236-241),落在 React commit 与该帧之间的 rejection(例如子 effect 中的 rejection —— DocumentErrorBoundary 无法捕获,React 边界不捕获 promise rejection)会通过 guard,app.textContent = … 将已提交的内容整体抹掉。

建议修复:给 showLoadError(detail) 增加原因参数 —— window-error 分支传入 event.message,措辞改为 renderer 崩溃(“Unable to display this chat export. The renderer failed after it loaded: …”);unhandledrejection 分支传入 event.reason;仅在 script 元素分支(event.target.id === 'transcript-renderer')保留现有 CDN/发布措辞。可选:当 #app 已有子节点时跳过 DOM 抹除。加载失败路径必须保留子串 'Unable to load this chat export'chat-transcript-document.test.ts:662 对 CDN 失败类断言了它(探针已确认 M2 修复形态与之兼容)。修复验收:新增浏览器门径用例,提供一个可执行且抛错的 body,并由测试将其 sha384 替换进 integrity 属性,断言 alert 文案不同于 CDN 缺失措辞且包含抛出的错误信息;移除 detail 传参后该用例必须变红(探针 M1/M2 已演示该翻转)。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +29 to +30
var showLoadError = function () {
if (document.body.dataset.renderComplete) return;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-3: (fix-induced) The renderComplete early-return guard added by the round-1 fix — the only thing preventing a late error/unhandledrejection event from clobbering an already-rendered transcript with the load-error screen — has no test: every error-path test fires before render completes (both fail-closed arms fault before React runs; the incompatible-envelope cases are handled inside the renderer). The round-1 report (handler only caught element-level fetch errors) is fixed by the new window-level wiring; this is the new defect the fix opened at the same site. The listeners stay attached for the page's lifetime, so if any post-render path ever rejects or errors, removing or reordering this guard replaces the rendered document with “Unable to load this chat export” — and no current test dispatches an error after data-render-complete='true', so that regression ships green.

Witness:

witness: not run — the guard-removal mutant was reasoned and the post-render-error probe written in a scratch tree, but chromium.launch() failed with ENOSPC (disk full) and an external sweep removed the trees; the coverage absence is static and complete — exhaustive greps confirm no test dispatches an error or rejection after data-render-complete='true'.

Suggested fix: add a browser-gate case — load a small document, wait for data-render-complete='true', then page.evaluate(() => Promise.reject(new Error('late probe'))) (and/or dispatch a synthetic window error), and assert data-render-complete is still 'true' and getByRole('alert') has count 0. The case must inject the error only after data-render-complete='true': the pre-render fail-closed semantics are pinned by the existing error-path tests (chat-transcript-document.test.ts:656-663 and 606-611), which require data-render-complete='error' plus the alert for failures before completion. Fix witness: that new test must go red when the early return is removed from showLoadError (the body flips to 'error').

中文说明

(修复引入)第一轮修复新增的 renderComplete 提前返回 guard —— 防止迟到的 error/unhandledrejection 事件用加载失败页面覆盖已渲染 transcript 的唯一防线 —— 没有测试:所有错误路径测试都在渲染完成前触发(fail-closed 的两个分支都在 React 运行前失败;不兼容 envelope 的用例在 renderer 内部处理)。第一轮的报告(handler 只捕获元素级 fetch 错误)已由新的 window 级链路修复;这是该修复在同一位置引入的新缺陷。listener 在页面整个生命周期内保持挂载,因此一旦任何渲染后路径出现 rejection 或错误,删除或调整这个 guard 就会把已渲染的文档替换为 “Unable to load this chat export” —— 而当前没有任何测试在 data-render-complete='true' 之后派发错误,所以这种回归会全绿通过。

建议修复:新增浏览器门径用例 —— 加载一个小文档,等待 data-render-complete='true',然后 page.evaluate(() => Promise.reject(new Error('late probe')))(和/或派发合成的 window error),断言 data-render-complete 仍为 'true'getByRole('alert') 数量为 0。该用例必须只在 data-render-complete='true' 之后注入错误:渲染前的 fail-closed 语义由现有错误路径测试钉住(chat-transcript-document.test.ts:656-663 与 606-611),它们要求完成前的失败产生 data-render-complete='error' 加 alert。修复验收:从 showLoadError 中移除该提前返回后,新测试必须变红(body 会翻转为 'error')。

— qwen3.8-max via Qwen Code /review (v0.23.0)

expect(html).not.toContain('qwen-code-assets.oss-cn-hangzhou.aliyuncs.com');
expect(html).not.toContain('type="importmap"');
expect(html).not.toContain('type="module"');
expect(html.length).toBeLessThan(500_000);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-5: Still stands (residual). Of the five renderer-content assertions this diff removes from html.test.ts, four (data-document-metadata, data-document-expand-all, data-document-collapse-all, data-document-theme-toggle) are now re-established — with stronger interactive coverage — in the browser gate's maximum-document test. 'Context Usage' is re-established nowhere: neither browser-gate fixture carries contextUsagePercent, the gate checks only [data-document-metadata] visibility, and the renderer only renders the <MetadataItem label="Context Usage" …> row when the value is defined (document-main.tsx:146-148, 181) — so that row is pinned by zero tests. A future change that drops, renames, or breaks the contextUsage computation ships silently: exported documents lose their context-usage metadata for every user while the static HTML test (string gone with the de-inlined bundle) and the visibility-only browser check stay green.

Witness:

witness: not run — nearest capability was a unit-harness mutant in a scratch tree (remove the Context Usage MetadataItem, run html.test.ts + browser gate), blocked by ENOSPC and an external tree sweep; the zero-pin claim is settled by exhaustive greps: 0 matches for 'Context Usage|contextUsage' across integration-tests (tests and fixtures), no test files under packages/web-templates/src/export-html/**, html.test.ts keeps contextUsagePercent: 25 in its fixture (line 25) but asserts nothing about its rendering.

Suggested fix: add contextUsagePercent (e.g. 25) to createMaximumDocument()'s metadata and assert the rendered content in the browser gate, e.g. expect(await page.locator('[data-document-metadata]').innerText()).toContain('Context Usage') — or assert the rendered '25%' value so the row's content, not just its label, is pinned. The max-document test enforces expect(envelopeBytes).toBeLessThanOrEqual(EXPORT_TRANSCRIPT_LIMITS_V1.maxEnvelopeBytes), so the added metadata must keep the envelope within budget (trivial for a number, but the assertion is live). Fix witness: the added browser-gate assertion must go red if the <MetadataItem label="Context Usage" …/> line at document-main.tsx:181 is removed.

中文说明

仍然成立(残余部分)。这个 diff 从 html.test.ts 移除的五个 renderer 内容断言中,四个(data-document-metadatadata-document-expand-alldata-document-collapse-alldata-document-theme-toggle)已在浏览器门径的最大文档测试中以更强的交互形式重建;但 'Context Usage' 没有任何地方重建:两个浏览器门径 fixture 都不携带 contextUsagePercent,门径只检查 [data-document-metadata] 的可见性,而 renderer 只在该值有定义时才渲染 <MetadataItem label="Context Usage" …> 行(document-main.tsx:146-148、181)—— 该行现在没有任何测试钉住。未来任何丢弃、重命名或破坏 contextUsage 计算的改动都会静默通过:所有用户的导出文档丢失上下文用量元数据,而静态 HTML 测试(字符串已随去内联消失)和仅检查可见性的浏览器断言都保持绿色。

建议修复:在 createMaximumDocument() 的 metadata 中加入 contextUsagePercent(例如 25),并在浏览器门径断言渲染内容,例如 expect(await page.locator('[data-document-metadata]').innerText()).toContain('Context Usage') —— 或直接断言渲染出的 '25%',钉住该行内容而不仅是标签。最大文档测试执行 expect(envelopeBytes).toBeLessThanOrEqual(EXPORT_TRANSCRIPT_LIMITS_V1.maxEnvelopeBytes),新增 metadata 必须保持 envelope 在预算内(一个数字微不足道,但该断言是活的)。修复验收:删除 document-main.tsx:181 的 <MetadataItem label="Context Usage" …/> 后,新增的浏览器门径断言必须变红。

— qwen3.8-max via Qwen Code /review (v0.23.0)

import { escapeJsonForHtmlScriptData } from '../packages/cli/src/ui/utils/export/html-script-data.js';

const RENDERER_VERSION = EXPORT_TRANSCRIPT_RENDERER_VERSION;
const RENDERER_URL = `https://unpkg.com/@qwen-code/qwen-code@${RENDERER_VERSION.split('+')[0]}/export-transcript-document.js`;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-9: Still stands. This suite lives outside every npm workspace and is collected only by the dedicated browser-gate legs (ci.yml:1421 web-shell E2E Smoke, e2e.yml:629) — never by any workspace npm test — and ci.yml's pull_request trigger covers only main/release/**, so this stacked PR (base codex/remove-webui) gets no test CI at all: gh pr checks 11035 at this round shows 17 checks with neither the Test (ubuntu-latest, Node 22.x) job nor web-shell E2E Smoke among them. If the renderer wiring regresses (wrong unpkg URL, missing integrity/crossorigin, an unexpected second network origin), every workspace suite and every CI check on this PR stays green while shipped exports fail to load the renderer in a browser.

Witness:

gh pr checks 11035 (this round): 17 checks; no 'Test (ubuntu-latest, Node 22.x)', no 'web-shell E2E Smoke'
ci.yml:21-24 pull_request branches: main, release/** — PR base is codex/remove-webui
browser gate collected only by ci.yml:1421 / e2e.yml:629; integration-tests is in no npm workspace

No code change — before merge, run the dedicated browser gate once PR-visibly (workflow_dispatch of ci.yml/e2e.yml on the head branch, or a local npx vitest run --root ./integration-tests ./chat-transcript-document.test.ts --retry=0 against a bundled build) and record the result in the PR thread; otherwise defer confidence explicitly to the follow-up PR targeting main.

中文说明

仍然成立。该套件位于所有 npm workspace 之外,只由专用浏览器门径 leg 收集(ci.yml:1421 web-shell E2E Smoke、e2e.yml:629),任何 workspace 的 npm test 都不会运行它;而 ci.yml 的 pull_request 触发只覆盖 main/release/**,因此这个堆叠 PR(base 为 codex/remove-webui)完全没有测试 CI:本轮 gh pr checks 11035 显示 17 个 check,其中既没有 Test (ubuntu-latest, Node 22.x) 也没有 web-shell E2E Smoke。如果 renderer 接线回归(unpkg URL 错误、缺少 integrity/crossorigin、出现第二个网络源),这个 PR 的所有 workspace 套件和所有 CI check 都会保持绿色,而已发布的导出在浏览器中将无法加载 renderer。

无需代码改动 —— 合并前请以 PR 可见的方式运行一次专用浏览器门径(在 head 分支上 workflow_dispatch ci.yml/e2e.yml,或对打包构建本地运行 npx vitest run --root ./integration-tests ./chat-transcript-document.test.ts --retry=0),并把结果记录到 PR 线程;否则请明确把信心推迟到 targeting main 的后续 PR。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment on lines +1921 to +1930
expect(
existsSync(
path.join(
extractDir,
'qwen-code',
'lib',
'export-transcript-document.js',
),
),
).toBe(false);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-10: Still stands. These new standalone-archive exclusion assertions run only via the root test:scripts script (ci.yml's Test job, line 759), outside every workspace's npm test — and because ci.yml's pull_request trigger covers only main/release/**, that job does not run on this stacked PR (gh pr checks 11035 at this round: 17 checks, no Test job). If scripts/create-standalone-package.js changes what it packs, a wrong inclusion/exclusion of the renderer in standalone release archives ships with every scoped workspace suite green and no CI check on this PR red; only the test:scripts lane would catch it, and it never runs here.

Witness:

gh pr checks 11035 (this round): 17 checks; no 'Test (ubuntu-latest, Node 22.x)' job
scripts/tests collected only via root test:scripts (package.json:52 → ci.yml:759)
ci.yml:21-24 pull_request branches: main, release/** — PR base is codex/remove-webui

No code change — run npm run test:scripts (or the whole test:ci lane) once on this head branch before merge and record the outcome in the PR thread.

中文说明

仍然成立。这些新增的 standalone 压缩包排除断言只通过根级 test:scripts 脚本运行(ci.yml 的 Test job,759 行),不在任何 workspace 的 npm test 内 —— 而由于 ci.yml 的 pull_request 触发只覆盖 main/release/**,该 job 不会在这个堆叠 PR 上运行(本轮 gh pr checks 11035:17 个 check,无 Test job)。如果 scripts/create-standalone-package.js 改变打包内容,standalone 发布包对 renderer 的错误包含/排除将在所有 scoped workspace 套件绿色、本 PR 无任何 CI check 变红的情况下发布;只有 test:scripts 通道能捕获,而它在这里从不运行。

无需代码改动 —— 合并前在该 head 分支上运行一次 npm run test:scripts(或整个 test:ci 通道),并把结果记录到 PR 线程。

— qwen3.8-max via Qwen Code /review (v0.23.0)

).toBe(true);
});

it('copies the HTML export renderer into the published package', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R1-11: Still stands. This test — pinning that copyBundleAssets + preparePackage ship export-transcript-document.js in the published npm package, the asset the whole unpkg/SRI design depends on — runs only via the root test:scripts script, outside every workspace's npm test, and that lane does not run on this stacked PR (ci.yml's pull_request trigger covers only main/release/**; gh pr checks 11035 at this round: 17 checks, no Test job). If scripts/copy_bundle_assets.js or scripts/prepare-package.js regresses (asset dropped from dist/ or from the package files list), all scoped workspace suites stay green and this PR's CI shows no failure; the breakage surfaces only at publish time as a 404/integrity-mismatch renderer load in every HTML export.

Witness:

gh pr checks 11035 (this round): 17 checks; no 'Test (ubuntu-latest, Node 22.x)' job
scripts/tests collected only via root test:scripts (package.json:52 → ci.yml:759)
ci.yml:21-24 pull_request branches: main, release/** — PR base is codex/remove-webui

No code change — confirm the test:scripts lane ran for this head branch before merge (locally or via workflow_dispatch) and record the result in the PR thread.

中文说明

仍然成立。这个测试 —— 钉住 copyBundleAssets + preparePackage 会把 export-transcript-document.js 随已发布的 npm 包一起发布(整个 unpkg/SRI 设计所依赖的资产)—— 只通过根级 test:scripts 脚本运行,不在任何 workspace 的 npm test 内,而该通道不会在这个堆叠 PR 上运行(ci.yml 的 pull_request 触发只覆盖 main/release/**;本轮 gh pr checks 11035:17 个 check,无 Test job)。如果 scripts/copy_bundle_assets.jsscripts/prepare-package.js 回归(资产从 dist/ 或包 files 列表中丢失),所有 scoped workspace 套件保持绿色、本 PR 的 CI 无任何失败;问题只会在发布时以每个 HTML 导出中 renderer 加载 404/完整性不匹配的形式暴露。

无需代码改动 —— 合并前确认 test:scripts 通道已在该 head 分支上运行(本地或通过 workflow_dispatch),并把结果记录到 PR 线程。

— qwen3.8-max via Qwen Code /review (v0.23.0)

qqqys pushed a commit to qqqys/qwen-code that referenced this pull request Sep 5, 2026
* feat(transcript): add cross-host document export pipeline

Establish a shared transcript model and document-mode projection so Web
Shell, VS Code, and HTML export can consume the same stable conversation
semantics without changing interactive rendering.

- Preserve daemon and ACP segment identity across replay and normalization
- Add export-safe previews and a versioned transcript document builder
- Add document-mode Web Shell rendering with bounded Mermaid processing
- Lock direct-daemon and ACP behavior with contract fixtures
- Cover render and export equivalence in integration tests

* feat(transcript): wire real VS Code and HTML export consumers

Move transcript prevalidation onto real product paths while preserving
the legacy VS Code timeline as the default fallback.

- Add version-bound document HTML with strict CSP and schema checks
- Add opt-in ACP projection with stable source identity
- Reuse product export code across hosts and the integration runner
- Remove test-only production probes and consolidate shared validation

Note: Browser, host-action, VSIX, and packaging gates remain pending.

* build(deps): sync transcript workspace dependencies

Keep the npm lockfile aligned with the VS Code and HTML export package
manifests added by the transcript consumer migration.

- Lock the VS Code Web Shell workspace dependency
- Lock Web Templates SDK, Web Shell, and React build dependencies

* feat(vscode-ide-companion): reuse WebShell transcript UI behind experimental flag

Bridge ACP session/update notifications into the shared SDK daemon transcript reducer and render the result with the WebShell transcript component, gated on qwen-code.experimental.webShellTranscript (default off).

The WebShell renderer and its heavy transitive dependencies (echarts, mermaid, shiki, codemirror, katex) are lazily loaded via esbuild code splitting, so the default configuration keeps the ~700KB webview bundle unchanged.

* fix(vscode-ide-companion): grant wasm-unsafe-eval for shiki WASM when WebShell transcript enabled

* fix(transcript): harden export and identity paths

Resolve review findings across document export and the VS Code ACP
timeline while preserving default interactive and readonly semantics.

- Preserve stable text and non-text identity across live and replay
- Harden export projection, budgets, URL handling, CSP, and nonces
- Make document rendering complete, inert, and browser-validated
- Keep VS Code transcript scope, theme, copy, and flags reactive
- Restore fixture, schema, hash, and compatibility contract locks

Note: The overall gate remains failed pending VSIX, host-action, and
packaged-artifact evidence.

* feat(vscode-ide-companion): adopt WebShell transcript as default timeline

Drop the experimental flag and the legacy MessageList renderer. The companion timeline now always renders through the shared WebShell transcript component, fed by ACP session/update notifications via the SDK daemon transcript reducer (lazy loaded through esbuild code splitting).

The flag-gated wiring is removed: the qwen-code.experimental.webShellTranscript setting, the conditional CSP/body attribute in WebViewContent, and the legacy MessageList path in App.tsx (~850 lines). The webview CSP now grants wasm-unsafe-eval unconditionally for Shiki's Oniguruma WASM.

* fix(vscode-ide-companion): reset WebShell transcript state on session switch

The experimental useAcpTranscript hook only consumed transcriptUpdate
messages, so its reducer state survived session boundaries. When the
extension switched sessions it kept the webview mounted and replayed the
newly-selected session through ACP, causing the previous session's blocks
to merge with the new replay (e.g. user text "alpha" from session A leaked
into session B as "alphabeta").

Reset both the reducer state and the rendered blocks on the same
boundaries the legacy message flow uses: qwenSessionSwitched (sent before
the ACP replay of the selected session) and conversationCleared (new
session). Adds a regression test that replays two sessions with a switch
between them.

* fix(vscode-ide-companion): harden WebShell transcript session boundaries

- reset the transcript on `conversationLoaded` too, closing the same
  cross-session leak the previous commit fixed for `qwenSessionSwitched`
  and `conversationCleared` (agent reconnect posts only this boundary)
- track the active session id and drop late `transcriptUpdate` frames
  whose `sessionId` no longer matches, so a previous session's trailing
  frames cannot contaminate the next session's timeline
- seed the transcript from cached messages carried by
  `qwenSessionSwitched` so offline restores and load-failure fallbacks
  render their history instead of a blank timeline
- dispatch `assistant.done` on `streamEnd`/`sessionLoadComplete` so the
  final assistant/thought block of a turn (or history replay) does not
  stay `streaming: true` forever

* fix(transcript): harden identity and document export

Close the latest review findings across transcript identity, VS Code
rehydration, and the HTML export security boundary.

- Terminate discrete ACP segments and bind automatic turn provenance
- Reset VS Code transcript scopes across replay and reconnect lifecycles
- Align Markdown sanitization and envelope budgets with document rendering
- Run browser gates with Chromium and complete third-party notices

* fix(vscode-ide-companion): adopt live ACP session id after load-failure fallback

* fix(vscode-ide-companion): echo user prompt into WebShell transcript

* fix(vscode-ide-companion): keep WebShell transcript expanded and clear of the composer

* fix(vscode-ide-companion): surface local error and interrupt notices in the transcript area

* fix(vscode-ide-companion): restore file-link opening from the WebShell transcript

* fix(vscode-ide-companion): restore contributed copy commands for the WebShell transcript

* fix(vscode-ide-companion): add localOnly marker to TextMessage state type

* fix(vscode-ide-companion): restore /insight progress card and report link in the transcript UI

* fix(vscode-ide-companion): finalize in-flight tools on timeout and pin session-switch seeding guard

Map streamEnd reasons timeout/session_expired onto the reducer's error reason so abandoned mid-tool turns no longer spin forever (ceuI). Add qwenSessionSwitched cases with no messages field and an empty cache array; the no-messages case fails when the seeding guard is forced true, pinning its false side (ceuN).

* fix(vscode-ide-companion): remove unreachable editMessage backend and dead submit options

The user-message edit/rewind UI was dropped in the WebShell-transcript migration, leaving editTargetTurnIndex/onSubmitted options in useMessageSubmit and the full editMessage/rewind flow in SessionMessageHandler unreachable. Remove the dead options, the editMessage dispatch case, the rewind/snapshot flow with its recovery branches, and their tests (R1-8 direction b).

* fix(vscode-ide-companion): drop write-only loadingMessage bookkeeping

The waiting-message renderer was removed with the WebShell transcript migration and the user prompt is echoed into the timeline at send time (bd09e19), so the loadingMessage string was write-only dead state. Keep the isWaitingForResponse flag (submit gating / cancel) and pin its API surface (R1-19 direction b).

* fix(vscode-ide-companion): align waiting-flag pin test with the argument-less setter

* fix(transcript): simplify adapters and preserve segment boundaries

Reduce the shared transcript review surface while keeping the VS Code and
HTML Export consumers and their security gates intact.

- Merge VS Code feature state into the ACP transcript hook
- Collect source identity once and strip it at the compatibility boundary
- Remove test-only export adapters, gate reports, and duplicate helpers
- Keep shell output separated when stable producer segments change
- Mark third-party notices as generated review content

* fix(transcript): address cross-host review regressions

* fix(vscode-ide-companion): echo attached images into the transcript timeline

The prompt carries pasted/attached images as ACP resource_link blocks,
which the transcript reducer cannot render (no inline data), so user
images vanished from the timeline while the attach path stayed alive.
Read each saved prompt image back from disk and echo it alongside the
text echo as an inline user_message_chunk image part (the daemon-echo
content shape), which the shared reducer folds into the user block and
the WebShell renderer already displays. Unreadable images are skipped
without breaking the send.

* fix(vscode-ide-companion): track live VS Code theme for the transcript

webShellTheme was snapshotted once at mount via useMemo with an empty
dependency array, so switching the VS Code color theme left the
timeline on the stale theme (VS Code updates data-vscode-theme-kind on
<body> in place without reloading the webview). Hold the theme in state
and refresh it with a MutationObserver on the body theme attributes.

* fix(vscode-ide-companion): copy every transcript block kind and map ambiguous row keys

- Copy All Messages now includes tool, shell, user_shell, and status
  blocks via getBlockCopyText, matching the pre-PR copyAllMessages
  handler which included formatted tool calls (review 5001842059 S-1).
- findBlockByRowKey prefers an exact id match and otherwise the longest
  matching block id, so one block id that dash-prefixes a sibling (e.g.
  `a` vs `a-1`) can no longer capture the sibling's row key (S-4).

* fix(vscode-ide-companion): drop whitespace-only cached transcript rows

cachedMessageToNotification rejected empty strings but admitted
whitespace-only content, which the reducer turns into an empty block
when seeding history from cached rows. Reject content that trims to
nothing (review 5001842059 S-2).

* fix(vscode-ide-companion): ship missing third-party notices in NOTICES.txt

Extend generate-notices.js so the regenerated NOTICES.txt carries the
attribution texts it previously only pointed at or dropped:

- Append license files from a package's licenses/ directory (echarts'
  Apache LICENSE references licenses/LICENSE-d3 for its embedded
  d3-derived files; the BSD-3-Clause text is now shipped).
- Append a package's NOTICE file when present (Apache-2.0 §4(d)),
  covering echarts' Apache Software Foundation attribution.
- Accept string-form package.json repository values (full URLs and
  GitHub shorthand) instead of emitting "(No repository found)".
- Fall back to the standard MIT text (copyright holder from package.json
  metadata) for MIT-declared packages that ship no license file.

* fix(vscode-ide-companion): show a recoverable error state when the transcript chunk fails to load

* test(vscode-ide-companion): gate the transcript blocks wiring into the WebShell renderer

* test(vscode-ide-companion): gate the transcriptUpdate forwarding from agent to webview

* docs(vscode): plan complete Web Shell cutover

* docs(webui): plan legacy package retirement

* docs(webui): link cutover prerequisite

* feat(vscode-ide-companion): reuse WebShell transcript UI behind experimental flag

Bridge ACP session/update notifications into the shared SDK daemon transcript reducer and render the result with the WebShell transcript component, gated on qwen-code.experimental.webShellTranscript (default off).

The WebShell renderer and its heavy transitive dependencies (echarts, mermaid, shiki, codemirror, katex) are lazily loaded via esbuild code splitting, so the default configuration keeps the ~700KB webview bundle unchanged.

* fix(vscode-ide-companion): grant wasm-unsafe-eval for shiki WASM when WebShell transcript enabled

* feat(vscode-ide-companion): adopt WebShell transcript as default timeline

Drop the experimental flag and the legacy MessageList renderer. The companion timeline now always renders through the shared WebShell transcript component, fed by ACP session/update notifications via the SDK daemon transcript reducer (lazy loaded through esbuild code splitting).

The flag-gated wiring is removed: the qwen-code.experimental.webShellTranscript setting, the conditional CSP/body attribute in WebViewContent, and the legacy MessageList path in App.tsx (~850 lines). The webview CSP now grants wasm-unsafe-eval unconditionally for Shiki's Oniguruma WASM.

* fix(vscode-ide-companion): reset WebShell transcript state on session switch

The experimental useAcpTranscript hook only consumed transcriptUpdate
messages, so its reducer state survived session boundaries. When the
extension switched sessions it kept the webview mounted and replayed the
newly-selected session through ACP, causing the previous session's blocks
to merge with the new replay (e.g. user text "alpha" from session A leaked
into session B as "alphabeta").

Reset both the reducer state and the rendered blocks on the same
boundaries the legacy message flow uses: qwenSessionSwitched (sent before
the ACP replay of the selected session) and conversationCleared (new
session). Adds a regression test that replays two sessions with a switch
between them.

* fix(vscode-ide-companion): harden WebShell transcript session boundaries

- reset the transcript on `conversationLoaded` too, closing the same
  cross-session leak the previous commit fixed for `qwenSessionSwitched`
  and `conversationCleared` (agent reconnect posts only this boundary)
- track the active session id and drop late `transcriptUpdate` frames
  whose `sessionId` no longer matches, so a previous session's trailing
  frames cannot contaminate the next session's timeline
- seed the transcript from cached messages carried by
  `qwenSessionSwitched` so offline restores and load-failure fallbacks
  render their history instead of a blank timeline
- dispatch `assistant.done` on `streamEnd`/`sessionLoadComplete` so the
  final assistant/thought block of a turn (or history replay) does not
  stay `streaming: true` forever

* fix(vscode-ide-companion): adopt live ACP session id after load-failure fallback

* fix(vscode-ide-companion): echo user prompt into WebShell transcript

* fix(vscode-ide-companion): keep WebShell transcript expanded and clear of the composer

* fix(vscode-ide-companion): surface local error and interrupt notices in the transcript area

* fix(vscode-ide-companion): restore file-link opening from the WebShell transcript

* fix(vscode-ide-companion): restore contributed copy commands for the WebShell transcript

* fix(vscode-ide-companion): add localOnly marker to TextMessage state type

* fix(vscode-ide-companion): restore /insight progress card and report link in the transcript UI

* fix(vscode-ide-companion): finalize in-flight tools on timeout and pin session-switch seeding guard

Map streamEnd reasons timeout/session_expired onto the reducer's error reason so abandoned mid-tool turns no longer spin forever (ceuI). Add qwenSessionSwitched cases with no messages field and an empty cache array; the no-messages case fails when the seeding guard is forced true, pinning its false side (ceuN).

* fix(vscode-ide-companion): remove unreachable editMessage backend and dead submit options

The user-message edit/rewind UI was dropped in the WebShell-transcript migration, leaving editTargetTurnIndex/onSubmitted options in useMessageSubmit and the full editMessage/rewind flow in SessionMessageHandler unreachable. Remove the dead options, the editMessage dispatch case, the rewind/snapshot flow with its recovery branches, and their tests (R1-8 direction b).

* fix(vscode-ide-companion): drop write-only loadingMessage bookkeeping

The waiting-message renderer was removed with the WebShell transcript migration and the user prompt is echoed into the timeline at send time (bd09e19), so the loadingMessage string was write-only dead state. Keep the isWaitingForResponse flag (submit gating / cancel) and pin its API surface (R1-19 direction b).

* fix(vscode-ide-companion): align waiting-flag pin test with the argument-less setter

* fix(vscode-ide-companion): echo attached images into the transcript timeline

The prompt carries pasted/attached images as ACP resource_link blocks,
which the transcript reducer cannot render (no inline data), so user
images vanished from the timeline while the attach path stayed alive.
Read each saved prompt image back from disk and echo it alongside the
text echo as an inline user_message_chunk image part (the daemon-echo
content shape), which the shared reducer folds into the user block and
the WebShell renderer already displays. Unreadable images are skipped
without breaking the send.

* fix(vscode-ide-companion): track live VS Code theme for the transcript

webShellTheme was snapshotted once at mount via useMemo with an empty
dependency array, so switching the VS Code color theme left the
timeline on the stale theme (VS Code updates data-vscode-theme-kind on
<body> in place without reloading the webview). Hold the theme in state
and refresh it with a MutationObserver on the body theme attributes.

* fix(vscode-ide-companion): copy every transcript block kind and map ambiguous row keys

- Copy All Messages now includes tool, shell, user_shell, and status
  blocks via getBlockCopyText, matching the pre-PR copyAllMessages
  handler which included formatted tool calls (review 5001842059 S-1).
- findBlockByRowKey prefers an exact id match and otherwise the longest
  matching block id, so one block id that dash-prefixes a sibling (e.g.
  `a` vs `a-1`) can no longer capture the sibling's row key (S-4).

* fix(vscode-ide-companion): drop whitespace-only cached transcript rows

cachedMessageToNotification rejected empty strings but admitted
whitespace-only content, which the reducer turns into an empty block
when seeding history from cached rows. Reject content that trims to
nothing (review 5001842059 S-2).

* fix(vscode-ide-companion): ship missing third-party notices in NOTICES.txt

Extend generate-notices.js so the regenerated NOTICES.txt carries the
attribution texts it previously only pointed at or dropped:

- Append license files from a package's licenses/ directory (echarts'
  Apache LICENSE references licenses/LICENSE-d3 for its embedded
  d3-derived files; the BSD-3-Clause text is now shipped).
- Append a package's NOTICE file when present (Apache-2.0 §4(d)),
  covering echarts' Apache Software Foundation attribution.
- Accept string-form package.json repository values (full URLs and
  GitHub shorthand) instead of emitting "(No repository found)".
- Fall back to the standard MIT text (copyright holder from package.json
  metadata) for MIT-declared packages that ship no license file.

* fix(vscode-ide-companion): show a recoverable error state when the transcript chunk fails to load

* test(vscode-ide-companion): gate the transcript blocks wiring into the WebShell renderer

* test(vscode-ide-companion): gate the transcriptUpdate forwarding from agent to webview

* docs(vscode): plan complete Web Shell cutover

* fix(transcript): harden export sanitization and user identity

Close the latest review findings around document resource safety and
recorded-user replay consistency.

- Redact local home paths structurally without corrupting remote URLs
- Sanitize nested Markdown images inside otherwise safe links
- Advance merged segment provenance without duplicating separators
- Keep recorded-user stable IDs anchored to durable record identity

* refactor(web-shell): own daemon React bindings

* fix(webui): preserve package entry filenames

* refactor(vscode): complete WebShell UI cutover

* chore(vscode): refresh third-party notices

* fix(vscode): fill embedded chat viewport

* test(web-shell): disambiguate workspace visual locator

* docs: clarify webui retirement prerequisites

* fix(transcript): resolve export and CI blockers

* fix(vscode): match embedded chat layout to host

* fix(vscode): compact embedded chat styling

* test(ci): cover dual Playwright installs

* fix(vscode): align embedded chat density with VS Code

* fix(vscode): complete embedded composer integration

* refactor: retire legacy webui package

* chore: refresh lockfile after webui removal

* fix(vscode): restore user message editing after cutover

* refactor: narrow webui retirement to export and removal

* fix(vscode): complete WebShell feature parity

* test(vscode-ide-companion): repair host-wiring tests for the WebShell cutover

* refactor(vscode-ide-companion): replace webui build scanner with an ESLint boundary rule

The bespoke recursive source scanner reimplemented a dependency-boundary
check on every extension build. A scoped no-restricted-imports rule
enforces the same boundary on every lint run with less custom code; the
manifest dependency entry was already removed by the cutover.

* fix(web-shell): keep ChatEditor commands prop referentially stable (QwenLM#9811)

The `additionalSlashCommands = []` destructure default allocated a fresh
array on every App render, invalidating the `commands` useMemo and breaking
ChatEditor memoization on every transcript-only re-render. Default to a
module-level constant instead, matching the existing EMPTY_* convention.

Also align the /skills completion expectation with the autoSubmit field the
completion source intentionally emits for leaf skill items.

* fix(vscode): distinguish the VS Code channel and localize its chrome

The companion now drives Web Shell against a shared `qwen serve` daemon,
so the CLI, the browser Web Shell, and this extension all create sessions
in the same workspace catalog. Web Shell recorded `'default'` for every
surface, leaving VS Code conversations indistinguishable from terminal and
browser ones — the panel's history listed sessions the user never opened
here, and nothing attributed a session back to the editor.

Give Web Shell a `sessionSourceType` prop (defaulting to today's
`'default'`) and have the companion stamp `'vscode'` on the sessions it
creates, then scope the history dropdown to that source. The host also
supplies a stable daemon `clientId`, which the bootstrap previously
declared but never sent.

Web Shell localizes its own surface from the `language` signal while the
companion's chrome was hardcoded English, so a zh-CN panel rendered a
Chinese transcript under an English header, history dropdown, onboarding
screen, and account dialog. Route that chrome through a small string table
driven by the same signal, including the host-only slash entries.

Also fix accessibility defects in the history dropdown: rename and delete
were revealed on hover alone and unreachable by keyboard, date headers sat
inside `role="listbox"` as invalid non-option children, arrow-key roving
stopped at group boundaries, `aria-modal` had no focus trap, and a primed
"Delete?" survived both search changes and the pointer leaving the row.

Formatting: `FileMessageHandler` and `SessionMessageHandler` were left
unformatted earlier in this branch and failed the Prettier gate.

* refactor(vscode): drop code orphaned by the WebShell cutover

The webview entry now renders EmbeddedApp against the daemon, which left
the ACP-era hook layer unreachable: nothing imports acpTranscriptAdapter,
useWebViewMessages, useAcpTranscript, useToolCalls, useSessionManagement,
useMessageHandling, useFileContext, useImage, or the permissionTypes added
by this branch. A reachability walk from webview/index.tsx reaches eight
modules; every reference to the rest comes from inside the orphaned set
itself, so it deletes as a closed unit.

EmbeddedWebShell goes with them. It was the host-driven entry point from
the earlier stage of this branch, superseded when EmbeddedApp moved to
WebShellWithProviders, and has had no consumer since — only its own DOM
test and a barrel export.

Also harden the daemon process lifecycle. `start()` returned the cached
runtime without comparing the workspace, so in a multi-root window the
second folder's chat silently reused a daemon bound to the first and
scoped every session, history page, and prompt to the wrong root. Bind the
daemon to its workspace and respawn on a change, keep a superseded child's
late exit from tearing down its successor, and report a post-startup exit
to the webview instead of leaving it fetching against a dead port.

* docs(vscode): describe the daemon architecture the cutover actually ships

The design doc still recorded the plan this branch started from: keep ACP
as the runtime boundary, add no daemon server or loopback port, and treat
"replacing ACP with daemon HTTP/SSE" as a non-goal. The final stage did
exactly that, so the document argued against the code beneath it.

Record the decision and its consequences instead — two processes per
workspace, a daemon shared with the CLI and browser Web Shell, the vscode
source type that keeps the panel's history its own, workspace rebinding in
multi-root windows, and the turn-driven host features that stopped firing.

* fix(vscode): repair round-2 review findings on the web-shell cutover (QwenLM#9811)

- closeDiff now resolves workspace-relative paths the same way showDiff
  does, so permission-cycle diffs opened from daemon-relative paths can
  actually be matched and closed
- a superseded or disposed daemon child no longer reports its exit as a
  crash of the live daemon
- authCancelled no longer hides an already-authenticated session behind
  onboarding; only an unknown auth state settles to unauthenticated
- selection-only activeEditorChanged events no longer undo an explicit
  active-file exclusion
- prepareSubmit dedupes mentions in both path spaces and matches typed
  references on a whole-reference boundary
- permission diffs open only from the SDK's authoritative file_diff
  preview (writes included, model-controlled toolCall mining removed)
- the webview HTML carries VS Code's locale so chrome strings localize
- discontinued qwen-oauth models are no longer re-applied through the
  new-session initial-model route

* fix(vscode): repair round-3 critical findings on the web-shell cutover (QwenLM#9811)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(web-shell): import daemon-react-sdk from web-shell instead of webui

The cutover branch dropped the ./daemon-react-sdk export from @qwen-code/webui,
but the TerminalPanel merged in from main still imports it, breaking the
web-shell vite build (Missing "./daemon-react-sdk" specifier). Point the import
and its test mock at @qwen-code/web-shell/daemon-react-sdk, which re-exports the
same useWorkspace hook and matches every other web-shell call site.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(vscode): close WebShell UI regression gaps

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(vscode): initialize WebShell refs explicitly

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(vscode): narrow queued prompt edits

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(release): enumerate actual npm workspaces

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(web-shell): include hasOlderHistory in the render-item callback deps

The renderItem useCallback reads hasOlderHistory to gate the edit action
but omitted it from its dependency array, failing CI's
react-hooks/exhaustive-deps gate.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(web-shell): report each connection error once to stop the inline onError re-render loop (QwenLM#10454)

* fix(web-shell): report each connection error once to stop the onError re-render loop

While a connection error persists (e.g. the daemon is unreachable), the
error-notification effect re-fires whenever the onError callback identity
changes. Hosts such as the VS Code embedded app pass an inline onError and
update their own state when it fires, so every notification triggers a host
re-render that hands the effect a fresh callback identity — re-notifying the
same persistent error forever (QwenLM#10406).

Track the last reported connection.error value in a ref and notify only when
the value changes, resetting the tracker once the connection recovers. This
guards every inline-callback consumer, not just memoized hosts.

Fixes QwenLM#10406

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(web-shell): only stamp the dedup ref once an onError handler exists

Stamping lastReportedConnectionErrorRef before delivery meant a host
that attaches onError after a persistent connection error appeared never
received it: the no-op delivery already marked the error as reported.
Guard on the handler first and add a regression test covering the
late-attach case (red when the guard is removed).

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(web-shell): document the onError dedup contract and fix comment wording

Describe the reported-once-per-distinct-error semantics, the reset on
recovery, and that replacing the handler mid-error does not re-deliver.
Reword the effect and test comments to describe the host class instead
of naming the VS Code embedded app, which passes a useCallback handler.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(vscode): mirror the web-shell value-dedup in the EmbeddedApp mock

The WebShellWithProviders mock re-notified on every onError identity
change, mirroring the loop App.tsx can no longer produce. Rewrite it to
report each distinct error value once (resetting on recovery), keep the
loop guard as a regression tripwire, exercise it with a changing
callback identity plus a post-delivery effect re-run, and refresh the
handleShellError comment that still cited the old loop as the
memoization reason.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(vscode): cast the captured onError prop for the mock wrapper

CapturedProps is an unknown index signature, so the destructured
onError needs the same cast the previous mock applied inline to stay
callable under tsc.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(vscode): bail the EmbeddedApp mock before stamping when no onError exists

The mirrored dedup effect stamped lastReportedError and counted a
notification even when no handler was attached, while App.tsx returns
before stamping on that path. Add the same early return so a handler
attached mid-error still receives the persistent error, and pin the
no-handler no-stamp behavior with a test that fails if the guard is
removed.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(web-shell): remove duplicate history dependency

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(vscode): close remaining WebShell cutover regressions

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(vscode): keep permission diff handling host-scoped

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(vscode): remove orphaned completion trigger test

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(web-shell): update daemon SDK mock import

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(export): reject unsupported legacy JSONL

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(transcript): record the legacy HTML renderer deletion evidence

The contract prevalidation doc is this design's single normative source
(§0.1), so retiring the legacy renderer has to be written back into it.
Three places had drifted:

- §2.1's consumer table still said the HTML Export path "keeps legacy
  compatibility for public calls without records". `toHtml` now requires
  records and the legacy branch is gone.
- §14's completion gate said the legacy HTML renderer is removed only once
  there is deletion evidence. New §12.4 records that evidence — no product
  consumer left, signature tightened, fallback explicitly rejected rather
  than silently degraded, and a `check:no-webui` guard against
  reintroduction — and keeps the VS Code legacy timeline half of the gate
  untouched.
- §10.7 promised credential removal without stating its scope. Spell out
  that it is http(s) only, and that non-navigable schemes, bare flag
  credentials and `code` / `inlineCode` nodes ship verbatim, so the
  document boundary is not read as a general secret scanner.

Also comment the two export format signatures: `records` is required
because HTML projects from original records, and the other formatters
ignore the argument.

Docs and comments only; no build or test was run locally.

* fix(docs): correct web-shell SDK hook names, daemon dir table, followup wiring

- daemon-client-adapters/web-shell.md: use the names the barrel actually
  exports (useActions/useConnection/usePendingPermissions/
  useTranscriptBlocks) in the import example and minimal React shape
- daemon/14-cli-tui-adapter.md: rewrite the
  packages/web-shell/client/daemon/ file table to the real layout
  (session/DaemonSessionProvider.tsx; the transcript adapter now lives
  in client/adapters/ and only exports extractPendingPermission)
- users/features/followup-suggestions.md: suggestion generation is
  automatic server-side; Web Shell hosts need no trigger wiring

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtngpuscjl

* fix(scripts): catch bare packages/webui references in check:no-webui

Widen the second forbidden pattern to /packages\/webui\b/g so
references followed by spaces, punctuation, or end-of-line are flagged
(CI step text, YAML list items, parenthesized mentions), while
lookalikes such as packages/webuix stay clean. Extract the detection
into containsForbiddenReference() and pin it with scripts/tests
fixtures, following the check-tui-dep-direction export convention.
The current tree still scans clean:
node scripts/check-no-webui-dependency.js exits 0.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtngpuscjl

* fix(docs): drop dead Adapter Matrix citation and stale Chromatic line

- 16-vscode-ide-adapter.md: 01-architecture.md has no "Adapter Matrix"
  section (headings verified at HEAD); drop the dead citation tail and
  keep the verified embedding claim (R1-11).
- terminal-capture/motivation.md: Chromatic retired with the webui
  package; point the "complement" item at the Playwright-based Web Shell
  visual tests already shown in this file's diagram (R1-12).

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtntktgsk8

* fix(comments): correct stale webui-era notes in SDK test and followup state

- daemonUi.test.ts: the referenced previewMarkdown/rawOutput preservation
  test does not exist in web-shell (transcriptAdapter.test.ts only covers
  extractPendingPermission); rewrite the note to say the enrichment path
  retired with the webui package instead of citing a nonexistent test
  (R1-6). Wording avoids the retired package literal so check:no-webui
  still passes.
- followupState.ts: no web-shell file imports this module (its daemon
  followup hook keeps its own FollowupState/controller; the only
  createFollowupController consumer is the CLI Ink hook); fix the module
  purpose comment and the createFollowupController docblock accordingly
  (R1-13).

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtntktgsk8

* fix(export): load transcript renderer from unpkg (QwenLM#11035)

* fix(export): load transcript renderer from CDN

* fix(export): harden CDN renderer loading

* fix(export): host transcript renderer on project OSS

* fix(export): serve npm renderer through unpkg

* fix(release): verify export renderer before VSIX packaging

* docs(daemon): retire the stale ACPAdapter and web-ui.md pointers

R1-2/R1-4 from the review round: the Consumers note still claimed the old
ACPAdapter host postMessage path "remains available" while this PR deletes
the only ACPAdapter, and the migration-status bullet still pointed at
web-ui.md after this PR renamed it to web-shell.md. Both lines now state
what ships: the postMessage path retired with packages/webui (webviews
embed Web Shell, matching 16-vscode-ide-adapter.md), and the adapter
design doc is web-shell.md.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs: avoid retired webui path reference

* fix: close WebUI retirement review gaps

---------

Co-authored-by: heyang.why <heyang.why@alibaba-inc.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: yiliang114 <jinjing.zzj@gmail.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants