Skip to content

feat: chat transcript mr2a html export - #10076

Open
water-in-stone wants to merge 24 commits into
QwenLM:mainfrom
water-in-stone:codex/feat-chat-transcript-mr2a-html-export
Open

feat: chat transcript mr2a html export#10076
water-in-stone wants to merge 24 commits into
QwenLM:mainfrom
water-in-stone:codex/feat-chat-transcript-mr2a-html-export

Conversation

@water-in-stone

Copy link
Copy Markdown
Collaborator

What this PR does

This PR delivers MR2A of the shared chat transcript design: a production HTML Export pipeline without migrating the VS Code live timeline.

  • Projects saved chat records through the existing canonical transcript semantics into a versioned, closed ExportTranscriptDocumentV1 with per-kind allowlists, opaque document IDs, typed tool previews/results, completeness diagnostics, and explicit resource budgets.
  • Renders the safe document through Web Shell document mode with full-document search, selection, copy, and print behavior; document-only Mermaid limits, safe fallbacks, and remote-resource restrictions do not change interactive or readonly runtime behavior.
  • Routes CLI, Web API, VS Code /export html, and the integration runner through the same version-bound product renderer while preserving the legacy HTML call when original records are unavailable.
  • Preserves existing interactive/readonly raw tool semantics, including complete write_file Turn Output diffs; document projection never falls back to raw payloads.
  • Keeps the VS Code live timeline on its existing legacy MessageList. MR2A adds no shared renderer dependency, transcript feature flag, live update forwarding, production adapter, or host-action seam; direct-daemon and ACP identity remain candidate evidence only, with selectedVscodePath: null and overall: "fail" until MR2B.

Why it's needed

The existing hosts have separate transcript and export paths, which risks duplicated replay/rendering behavior and inconsistent portable output. HTML export also needs a one-way security boundary so raw tool payloads, credentials, absolute user paths, session identity, unsafe resources, and unbounded rendering work cannot enter a shareable document. MR2A establishes that bounded document path independently from the higher-risk VS Code live timeline migration, allowing the two changes to be reviewed, rolled out, and reverted separately.

Reviewer Test Plan

How to verify

  1. Run npm run build and npm run typecheck; expect the SDK, CLI, Web Shell library, embedded document renderer, and VS Code companion export consumer to build and type-check while VS Code continues to use its legacy live timeline.
  2. Run npx vitest run --root ./integration-tests ./chat-transcript-contract.test.ts --retry=0; expect fixture hashes, the production schema, raw-free document semantics, and both candidate identity projections to pass while selectedVscodePath remains null and the overall gate remains fail.
  3. Run the focused SDK, CLI Export, Web Shell document-mode, and VS Code session-export tests; expect typed preview/result rendering in document mode, unchanged interactive/readonly raw behavior, complete write_file Turn Output diffs, and VS Code /export html to pass original records into the shared document pipeline.
  4. Generate HTML through the CLI, Web API, VS Code /export html, and the integration runner; expect the same version-bound, self-contained renderer and a safe read-only failure for incompatible schema or renderer versions.
  5. Run the product HTML browser gate with Chromium; expect maximum-budget open/search/copy/print behavior, zero unexpected network requests, zero CSP violations, no canary leakage, and safe degradation for remote images, unsafe Markdown/HTML, Mermaid, code, and diff failures.
  6. Regenerate VS Code third-party notices; expect no transcript-related dependency or NOTICES.txt delta in MR2A because the live Web Shell timeline dependency is deferred to MR2B.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Risk & Scope

  • Main risk or tradeoff: The document projector and renderer are a security boundary; under-sanitization could leak sensitive data or trigger network activity, while over-sanitization could remove valid transcript content. Shared renderer changes must also remain isolated from interactive and readonly modes.
  • Not validated / out of scope: VS Code live transcript migration, product-path selection, scope/generation admission, reconnect and late-update handling, host-action parity, VSIX and installed-artifact packaging, Desktop installed-artifact smoke, OpenWork/Craft Electron, and OS-specific validation are deferred or not provided.
  • Breaking changes / migration notes: No live VS Code timeline migration is included; the legacy MessageList remains authoritative. Interactive/readonly raw semantics remain the compatibility baseline, document-only safe projection does not flow back into live sessions, and the migration gate intentionally remains overall: "fail" with selectedVscodePath: null.

Linked Issues

Linked #9387

中文说明

What this PR does

本 PR 交付共享聊天 transcript 设计的 MR2A:落地生产级 HTML Export pipeline,但不迁移 VS Code live timeline。

  • 将已保存的聊天记录通过现有 canonical transcript 语义投影为版本化、封闭的 ExportTranscriptDocumentV1,包含逐 kind allowlist、文档内 opaque ID、typed tool preview/result、完整性 diagnostic 和显式资源预算。
  • 通过 Web Shell document mode 渲染安全文档,支持全文搜索、选择、复制和打印;仅 document mode 生效的 Mermaid 限制、安全 fallback 和远程资源约束不会改变 interactive 或 readonly runtime 行为。
  • 让 CLI、Web API、VS Code /export html 和 integration runner 复用同一个版本绑定的产品 renderer;无法取得原始 records 时仍保留 legacy HTML 调用兼容性。
  • 保持现有 interactive/readonly raw 工具语义,包括完整的 write_file Turn Output diff;document projection 永不回退 raw payload。
  • VS Code live timeline 继续使用现有 legacy MessageList。MR2A 不新增 shared renderer 依赖、transcript feature flag、live update 转发、生产 adapter 或 host-action seam;direct-daemon 与 ACP identity 仅作为候选证据保留,在 MR2B 前维持 selectedVscodePath: nulloverall: "fail"

Why it's needed

现有宿主拥有不同的 transcript 与 export 路径,存在重复 replay/rendering 行为和可移植输出不一致的风险。HTML export 还需要一个单向安全边界,防止 raw tool payload、credential、绝对用户路径、session identity、危险资源和无界渲染工作进入可分享文档。MR2A 将该有界 document path 与风险更高的 VS Code live timeline 迁移分离,使两者可以独立评审、发布和回滚。

Reviewer Test Plan

How to verify

  1. 运行 npm run buildnpm run typecheck;预期 SDK、CLI、Web Shell library、内嵌 document renderer 和 VS Code companion export consumer 均可构建并通过类型检查,同时 VS Code 继续使用 legacy live timeline。
  2. 运行 npx vitest run --root ./integration-tests ./chat-transcript-contract.test.ts --retry=0;预期 fixture hashes、生产 schema、raw-free document semantics 和两条 candidate identity projection 均通过,同时 selectedVscodePath 保持 null、overall gate 保持 fail
  3. 运行聚焦的 SDK、CLI Export、Web Shell document-mode 和 VS Code session-export 测试;预期 document mode 使用 typed preview/result 渲染,interactive/readonly raw 行为不变,write_file Turn Output diff 保持完整,并且 VS Code /export html 将原始 records 传入共享 document pipeline。
  4. 分别通过 CLI、Web API、VS Code /export html 和 integration runner 生成 HTML;预期使用同一个版本绑定、自包含的 renderer,并在 schema 或 renderer version 不兼容时安全地只读失败。
  5. 使用 Chromium 运行产品 HTML browser gate;预期最大预算文档可执行 open/search/copy/print,unexpected network requests 为零、CSP violations 为零、无 canary 泄漏,并对远程图片、危险 Markdown/HTML、Mermaid、code 和 diff 失败进行安全降级。
  6. 重新生成 VS Code third-party notices;预期 MR2A 不引入 transcript 相关依赖或 NOTICES.txt diff,因为 live Web Shell timeline 依赖已延后到 MR2B。

Evidence (Before & After)

未提供 screenshot 或 recording。输入文档记录了可重复 fixtures、SHA-256 锁、capability matrix、聚焦的 SDK/Core/CLI/Web Shell/VS Code export 覆盖、direct-daemon/ACP candidate checks、产品 HTML build assertions,以及覆盖最大文档、主动 CSP enforcement、零意外网络请求、搜索、复制、打印、远程资源降级和时间戳归一化的 Chromium browser gate。

Tested on

OS Status
🍏 macOS ⚠️ not tested — 输入文档未注明操作系统
🪟 Windows ⚠️ not tested — 输入文档未注明操作系统
🐧 Linux ⚠️ not tested — 输入文档未注明操作系统

Environment (optional)

N/A — 输入文档未注明 runtime environment 或 sandbox。

Risk & Scope

  • Main risk or tradeoff: document projector 和 renderer 是安全边界;脱敏不足可能泄漏敏感数据或触发网络活动,过度脱敏则可能删除有效 transcript 内容。共享 renderer 改动还必须与 interactive 和 readonly mode 保持隔离。
  • Not validated / out of scope: VS Code live transcript migration、产品路径选择、scope/generation admission、reconnect 与迟到 update 处理、host-action parity、VSIX 与 installed-artifact packaging、Desktop installed-artifact smoke、OpenWork/Craft Electron 和 OS-specific validation 均已延后或未提供证据。
  • Breaking changes / migration notes: 本 PR 不包含 VS Code live timeline 迁移;legacy MessageList 继续作为权威路径。interactive/readonly raw 语义仍是兼容性基线,document-only safe projection 不会回流 live session,并且 migration gate 有意维持 overall: "fail"selectedVscodePath: null

Linked Issues

#9387

N/A


heyang.why added 17 commits August 20, 2026 15:26
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
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.
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
…plit-20260818

# Conflicts:
#	packages/acp-bridge/src/transcript-replay.ts
#	packages/sdk-typescript/scripts/build.js
#	packages/sdk-typescript/src/daemon/ui/transcript.ts
#	packages/web-shell/client/components/MessageList.tsx
#	packages/web-shell/client/components/messages/GoalStatusMessage.tsx
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.
…eb-shell-adapter-before-split-20260818

# Conflicts:
#	packages/web-shell/client/components/MessageList.tsx
#	packages/web-shell/client/components/messages/AssistantMessage.tsx
#	packages/web-shell/client/components/messages/ToolGroup.tsx
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
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
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
…plit-20260818

# Conflicts:
#	packages/cli/src/acp-integration/session/Session.ts
#	packages/web-shell/client/components/MessageList.tsx
#	packages/web-shell/client/components/WebShellTranscript.tsx
#	packages/web-shell/client/components/messages/ToolGroup.test.tsx
#	packages/web-shell/client/components/messages/ToolGroup.tsx
Keep MR2A focused on the document export pipeline while preserving the legacy VS Code timeline.

- Retain VS Code /export html as a real document consumer
- Move live transcript adapters, identity wiring, and feature flag to MR2B
- Remove the direct Web Shell dependency and restore baseline notices
- Keep candidate identity evidence in integration-only helpers
- Document the MR1, MR2A, and MR2B delivery boundaries
# Conflicts:
#	packages/web-shell/client/components/messages/ToolGroup.tsx
# Conflicts:
#	packages/web-shell/client/components/WebShellTranscript.tsx
#	packages/web-shell/client/components/messages/PlanMessage.test.tsx
#	packages/web-shell/client/components/messages/TodoView.tsx
#	packages/web-shell/client/components/messages/ToolGroup.tsx
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head df02ec3, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

No response changes against the PR base across 12 scenario(s).

Qwen Code · serve A/B

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head df02ec3. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

⚠️ No preview: one or more scenarios failed to render on this head — see the workflow run. This is not "no visual change" — a scenario that times out or throws produces no image. Fix the failing scenario (or a genuine regression it caught) and the preview returns on the next push.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 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! This is MR2A of the shared chat transcript design — a planned milestone, and it reads that way. Gate notes below.

Template ✓ — all required sections are present. One small nit: the English half omits the ### Evidence (Before & After) subheading that the Chinese half includes. Not blocking.

Problem: this is a planned feature milestone, not theoretical hardening. It implements the merged design doc docs/design/web-shell/chat-transcript-contract-prevalidation.md (§9.6 HTML Export pipeline, §12.2 MR2 steps 5–9), follows the already-merged prevalidation stage (#9388), and links issue #9387. Worth noting: #9387 was opened by the same author and still carries need-discussion / priority/P3 — the prevalidation stage got merged, but the issue thread itself never got an explicit maintainer green-light for the production migration.

Direction: aligned. The design doc — merged by a maintainer in #9388 on 2026-08-19 — prescribes exactly this split: export builder + document mode + HTML wiring + browser/security gates now, VS Code live timeline migration deferred to MR2B with the gate intentionally left at overall: "fail" and selectedVscodePath: null. The issue carries roadmap/export-data. Claude Code's changelog has no direct HTML-export entry, but transcript rendering is an actively maintained surface there — the area is relevant.

Size: large. 11,210 changed lines across 96 files and 7 packages (cli, core, sdk-typescript, acp-bridge, web-shell, web-templates, vscode-ide-companion) — of which ~6,101 are production logic (68 files), 5,000 are tests (27 files), and 109 are schema. For a feat PR there is no hard block, but this carries the maintainer-awareness flag at 500+ production lines and the large-PR advisory at 1000+: if any seam here can be split (e.g. document schema + projector separately from host wiring), two smaller PRs would review faster. Deferring MR2B already helps.

Approach: the scope matches the design doc's consumer-ordered plan — the centerpiece is the new closed ExportTranscriptDocumentV1 projector (~2,000 lines under packages/cli/src/ui/utils/export/), plus markdown document policy, the web-templates document entry, and the integration-runner rewrite (−894 lines converging onto the shared pipeline). Things worth thinking about: the CLI-only build path now also compiles webui/web-shell/web-templates (build-time cost on every fast lane), and four new runtime dependencies (unified, remark-parse, remark-gfm, remark-math) enter the CLI for the markdown policy. Both follow from the design, but they are real tradeoffs.

Risk: no matches on the revert-correlated high-risk paths. Elevated attention goes to two places instead: the shared web-shell components (also serving the live interactive shell) and the security boundary itself — CI evidence will matter, and the ubuntu lane is currently red (details in the review comment).

Moving on to code review. 🔍

中文说明

感谢贡献!这是共享 chat transcript 设计的 MR2A——一个有计划的里程碑,整体也是如此呈现的。

模板 ✓ — 所有必需章节齐全。小问题:英文部分缺少中文部分包含的 ### Evidence (Before & After) 小节。不构成阻塞。

问题:这是有计划的特性里程碑,不是理论性加固。它实现了已合并的设计文档 chat-transcript-contract-prevalidation.md(§9.6 HTML Export 管线、§12.2 MR2 第 5–9 步),继已合并的 prevalidation 阶段(#9388)之后,并关联 issue #9387。值得注意:#9387 由同一作者提出,仍带 need-discussion / priority/P3 标签——prevalidation 阶段已合并,但 issue 本身尚未得到维护者对生产迁移的明确认可。

方向:对齐。设计文档(由维护者在 #9388 于 2026-08-19 合并)恰好规定了这一拆分:先做 export builder + document mode + HTML wiring + 浏览器/安全门禁,VS Code live timeline 迁移推迟到 MR2B,门禁有意保持 overall: "fail"selectedVscodePath: null。issue 带有 roadmap/export-data 标签。claude-code 的 changelog 没有直接的 HTML export 条目,但 transcript 渲染在那里是活跃维护的领域——该方向是相关的。

规模:大。96 个文件、7 个包共 11,210 行改动,其中约 6,101 行为生产逻辑(68 个文件),5,000 行为测试(27 个文件),109 行为 schema。feat 类 PR 没有硬性阻断,但 500+ 生产行触发维护者关注、1000+ 触发大 PR 建议:如果存在可拆分的接缝(例如 document schema + projector 与宿主接线分开),两个更小的 PR 审起来更快。推迟 MR2B 已经有所缓解。

方案:范围符合设计文档按消费者排序的计划——核心是新的封闭式 ExportTranscriptDocumentV1 投影器(约 2,000 行,位于 packages/cli/src/ui/utils/export/),外加 markdown 文档策略、web-templates document 入口,以及 integration runner 重写(−894 行,收敛到共享管线)。值得思考:CLI-only 构建路径现在也要编译 webui/web-shell/web-templates(每个快速通道的构建时间成本),并且 CLI 新增四个运行时依赖(unifiedremark-parseremark-gfmremark-math)用于 markdown 策略。两者都源自设计,但都是真实的取舍。

风险:未命中与 revert 相关的高风险路径。需要重点关注两处:共享 web-shell 组件(同时服务于 live 交互界面)和安全边界本身——CI 证据很重要,而 ubuntu 通道当前是红的(详见审查评论)。

进入代码审查。🔍

Qwen Code · qwen3.8-max

Reviewed at 5f9470b974c2e60e33f0d3278566adf7a69669f1 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

I read the security boundary in detail — it is the heart of this PR, and it holds up: per-kind Pick allowlists with assertNever exhaustiveness (a future block kind fails the build, not the user), forbidden-field scans (rawInput/rawOutput/sessionId/…), fail-closed validateStrict in core, then a second full re-validation of the finished document (semantic safety + consistency + budgets + schema). Opaque IDs, zeroed block timestamps, home-path redaction, URL-scheme allowlist, data-URI-only images (no SVG), CSP default-src 'none' with nonce-gated scripts, renderer-version binding, and a build that refuses to inline any <script sequence. Live-mode preservation is structural, not hopeful: document is a third render mode, adapter behavior is opt-in flagged (safeToolProjection, includeSourceIdentity, default off), and the MessageList change is an extraction of groupParallelAgents into a shared adapter. The pipeline reuses the SDK's canonical projector instead of re-replaying records, per design §9.6. No critical security or correctness holes found in the boundary itself.

What does need attention:

  1. CI is red on this head, and the failure is in code this PR restructured. The ubuntu lane fails one web-shell test (evidence below): MessageList — compact mode > keeps a folded single-agent summary separate from an approving agent — the expected msg-summary-t1 node never renders. That test exists unchanged on main and is not modified here, while the grouping logic it exercises is exactly what this PR extracted and retyped. Please fix it or demonstrate it is a flake with a re-run; as it stands it contradicts the "interactive/readonly behavior preserved" claim.
  2. The SDK preview path changed semantics beyond export. segmentId now gates block merging (user text gets \n separators across segments, shell blocks only merge on matching segment), and argsSummary generation now redacts sensitive keys — on the shared daemon preview path, so the live UI sees these too. The updated 520-line daemonUi suite pins the new behavior, but the PR describes itself as preserving runtime semantics; worth a maintainer confirming that is the intended scope.
  3. Build/runtime footprint tradeoffs (follow the design, but are real): --cli-only builds now compile webui + web-shell + web-templates, and four new CLI runtime deps (unified, remark-parse, remark-gfm, remark-math) serve the markdown policy. Minor: Mermaid renders now queue through one global promise even in non-document mode.
sequenceDiagram
    participant P1 as ChatRecord records
    participant P2 as Record export policy
    participant P3 as SDK canonical projector
    participant P4 as Document projector
    participant P5 as Gates
    participant P6 as CSP template
    participant P7 as Browser gate
    P1->>P2 keep visible record kinds only
    P2->>P3 project to transcript blocks
    P3->>P4 typed allowlist, opaque IDs, redaction, budgets
    P4->>P5 schema plus semantic re-validation, fail closed
    P5->>P6 inject escaped JSON under nonce CSP
    P6->>P7 zero network, canary, search copy print probes
Loading
Files changed (30 of 96 shown)
File What changed
packages/cli/src/ui/utils/export/export-transcript-document.ts Centerpiece: closed V1 projector with per-kind allowlists, budgets, opaque IDs, redaction, double assertion pass
packages/cli/src/ui/utils/export/export-transcript-document-v1.schema.json Schema moved from test fixtures into production and tightened
packages/cli/src/ui/utils/export/markdown-document-policy.ts remark-based markdown URL and image policy for document text
packages/cli/src/ui/utils/export/formatters/html.ts Document render entry: renderer version check, CSP nonce, script-data injection, legacy fallback
packages/cli/src/ui/utils/export/html-script-data.ts JSON-in-script escaping extracted to a shared helper
packages/cli/src/ui/commands/exportCommand.ts CLI /export html now passes original records into the new pipeline
packages/cli/src/serve/server/session-export.ts Serve API passes original records for the html format
packages/vscode-ide-companion/src/services/sessionExportService.ts VS Code /export html routes records into the shared pipeline
packages/core/src/utils/schemaValidator.ts Additive validateStrict: no coercion, no fail-open, no mutation
packages/sdk-typescript/src/daemon/ui/toolPreview.ts Typed todo and result previews, argsSummary sensitive-key redaction
packages/sdk-typescript/src/daemon/ui/transcript.ts segmentId propagation and changed merge rules on the shared reducer
packages/sdk-typescript/src/daemon/ui/normalizer.ts segmentId and resultPreview extraction from event metadata
packages/sdk-typescript/src/daemon/ui/types.ts Preview and identity type additions
packages/web-shell/client/transcriptRenderMode.ts Adds the document render mode value
packages/web-shell/client/components/WebShellTranscript.tsx Document-mode wiring: no virtualization, no collapse, basic tables
packages/web-shell/client/adapters/transcriptToMessages.ts Opt-in safeToolProjection and source identity flags, default off
packages/web-shell/client/adapters/parallelAgentGrouping.ts Parallel-agent grouping extracted here to be shared with document mode
packages/web-shell/client/components/MessageList.tsx Consumes the extracted grouping; display-item types change (source of the red test)
packages/web-shell/client/components/messages/Markdown.tsx Document-only Mermaid limits and timeout, data-URI-only images in document mode
packages/web-templates/src/export-html/build.mjs Second bundle entry; refuses script-sequence inlining; placeholder guards
packages/web-templates/src/export-html/src/document-index.html CSP default-src none with nonce slots, no remote resources
packages/web-templates/src/export-html/src/document-main.tsx Document renderer bootstrap bound to the renderer version
packages/acp-bridge/src/transcript-replay.ts Replay adjustments feeding the document fixtures
integration-tests/chat-transcript-contract.test.ts Contract gate updated, overall stays fail with null selected path
integration-tests/chat-transcript-document.test.ts New Chromium browser gate: zero network, CSP, canary, search, copy, print
integration-tests/concurrent-runner/export-html-from-chatrecord-jsonl.js Runner rewritten onto the shared pipeline, minus 894 lines
.github/workflows/ci.yml Dedicated browser-gate step; playwright install paths de-duplicated
.github/workflows/e2e.yml Browser gate excluded from sharded e2e lanes
scripts/build.js cli-only builds now also build webui, web-shell, web-templates
docs/design/web-shell/chat-transcript-contract-prevalidation.md Design doc status updated for MR2A
…and 66 more files unit tests, fixtures, capability matrix, CSS modules, package manifests

Test evidence (this PR's own CI — unattended run, no PR code executed here)

The PR's central lane is red on the reviewed commit. Test (ubuntu-latest, Node 22.x) fails in the web-shell suite: 1 failed / 4276 passed (4277)

FAIL components/MessageList.dom.test.tsx
  > MessageList — compact mode > keeps a folded single-agent summary separate from an approving agent
AssertionError: expected null not to be null
 ❯ components/MessageList.dom.test.tsx:736:11
    expect(container.querySelector('[data-testid="msg-summary-t1"]')).not.toBeNull();

I verified this test exists at line 718 of MessageList.dom.test.tsx on main and is not touched by this PR, while the grouping code it exercises is exactly what this PR extracted and retyped — so I classify it as PR-caused rather than infra noise (log body claims aside, the diff and check identity say otherwise). The job log ends with the runner terminating after the suite; the failure above is the exit cause. macOS and Windows unit lanes are skipped on PRs by design (#10059); the integration lane was skipped for this run. Twenty auxiliary lanes passed, including web-shell E2E Smoke, Serve A/B, Desktop Shell (ubuntu + windows), Dependency CVE audit, Secret scan (TruffleHog), and precheck-pr. Not independently verified here: the author's reviewer-test-plan claims (Chromium browser gate behavior, VSIX notices parity) — the browser gate produced no green signal in this commit's CI (the lane failed at the unit step).

Check Conclusion
Test (ubuntu-latest, Node 22.x) ❌ failure — 1/4277 web-shell tests: MessageList compact-mode summary
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Serve A/B (ubuntu-latest, Node 22.x) ✅ success
Desktop Shell (ubuntu-22.04 / windows-2022) ✅ success
Dependency CVE audit / Secret scan / precheck-pr ✅ success
Test (macos-latest / windows-latest, Node 22.x) ⏭️ skipped (off PRs per #10059)
Integration Tests (CLI, No Sandbox) ⏭️ skipped

Sandboxed verification would settle the behavioural claims once CI is green: @qwen-code /verify — whether the new document pipeline is load-bearing (A/B: export HTML through the new projector vs the legacy path, and does the browser gate actually fail when the allowlist projection is removed) cannot be proven by the unit suite alone; @qwen-code /tmux — for the user-visible /export html TUI surface. The author has write access, so either lane can be triggered directly; until the red unit test is resolved neither would say much.

中文说明

代码审查:安全边界是本 PR 的核心,审查结论是站得住的——逐 kind 的 Pick 白名单加 assertNever 穷尽检查(未来新增 block 类型会在构建期失败而不是在用户侧失败)、禁止字段扫描、核心层的 fail-closed validateStrict,以及对最终文档的第二轮完整复验(语义安全 + 一致性 + 预算 + schema)。opaque ID、块时间戳归零、家目录路径脱敏、URL 协议白名单、仅 data-URI 图片(无 SVG)、CSP default-src 'none' + nonce 脚本、渲染器版本绑定、构建期拒绝内联任何 <script 序列。live 模式的保持是结构性的:document 是第三种渲染模式,适配器行为由 opt-in 开关控制(默认关闭),MessageList 的改动只是把 groupParallelAgents 抽取为共享适配器。管线复用了 SDK 的 canonical projector,符合设计 §9.6。边界本身未发现严重安全或正确性漏洞。

需要关注的:

  1. 该 head 上 CI 是红的,且失败点正是本 PR 重构的代码。 ubuntu 通道有一个 web-shell 测试失败(证据见下):期望的 msg-summary-t1 节点没有渲染。该测试在 main 上原样存在且本 PR 未修改,而它测试的分组逻辑正是本 PR 抽取并重新定义类型的部分。请修复,或用重跑证明是 flake;当前状态与"保持 interactive/readonly 行为"的声明相矛盾。
  2. SDK 预览路径的语义变化超出了 export 范围。 segmentId 现在控制块合并,argsSummary 生成现在会脱敏敏感键——这发生在共享的 daemon 预览路径上,live 界面同样可见。更新后的 520 行 daemonUi 测试固定了新行为,但 PR 自述为保持运行时时序,值得维护者确认这是否预期范围。
  3. 构建/运行时足迹取舍(符合设计但真实存在):--cli-only 构建现在也要编译 webui + web-shell + web-templates;CLI 新增四个运行时依赖用于 markdown 策略。次要:Mermaid 渲染在非 document 模式下也通过全局队列串行化。

测试证据(引用本 PR 自身 CI,本次为无人值守运行,未执行任何 PR 代码):核心通道 Test (ubuntu-latest, Node 22.x) 在被审提交上是红的——web-shell 套件 1 失败 / 4276 通过,失败测试在 main 上原样存在且未被本 PR 修改,判定为 PR 导致而非基础设施噪音。macOS/Windows 单元通道按 #10059 设计对 PR 关闭;二十条辅助通道通过(含 web-shell E2E Smoke、Serve A/B、Desktop Shell、CVE 审计、密钥扫描)。作者自述的浏览器门禁行为未在此独立验证——浏览器门禁挂在失败的单测步骤之后,本次提交未跑完。CI 绿后建议用 @qwen-code /verify(A/B 证明 document 管线真正承重)与 @qwen-code /tmux/export html TUI 面)做沙箱验证;作者有写权限,可直接触发。

Qwen Code · qwen3.8-max

Reviewed at 5f9470b974c2e60e33f0d3278566adf7a69669f1 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 2/5 — the security boundary is genuinely well built, but the PR's own CI fails a pre-existing test that pins the exact "live behavior preserved" claim it makes, and a change of this size needs a maintainer in the loop regardless.

Stepping back: my independent take on this problem would have been almost exactly what this PR does — a closed, versioned document type with per-kind allowlists, a one-way projector that never serializes raw payloads, reuse of the existing canonical projection, a nonce-CSP template with zero remote surface, and all hosts converged on one renderer. The implementation matches or exceeds that, and the defensive layers (forbidden-field scan, second full re-validation, assertNever exhaustiveness, build-time refusal to inline script sequences) are better than what I would have sketched. This is not volume-for-volume's-sake; it tracks a maintainer-merged design doc step by step.

But two things keep this from a clean pass:

  • The red CI is the core claim breaking. The failing test is unchanged from main and covers the compact-mode grouping this PR extracted and retyped. If the extraction were truly behavior-preserving, that test would pass. Either the extraction changed something subtle (my read: likely) or the test is flaky (prove it with a re-run). Until then, "interactive/readonly semantics preserved" is asserted, not demonstrated — and on a security-boundary PR, the compatibility claim carries the same weight as the boundary itself.
  • Scope for a human, not a gate. ~6,100 production lines across 7 packages, a shared SDK preview path whose semantics changed (segmentId merge rules, argsSummary redaction), and a CLI build/runtime footprint that grows even under --cli-only. Nothing here looks wrong on reading, but at this size "looks right on reading" is not the bar — the Stage 0 rule puts this in maintainer territory, and I am capping my confidence accordingly rather than approving on the strength of a clean-ish read.

Verdict: request changes on the red CI (fix or prove flake), and defer the rest to @yiliang114 — you merged the prevalidation stage (#9388) this builds on; the questions worth your eyes are the SDK preview-semantics scope and whether MR2A should land whole or split at the schema/projector seam. @water-in-stone strong work on the boundary itself; the blocker is narrow and concrete.

中文说明

置信度:2/5 — 安全边界构建得确实很好,但 PR 自身的 CI 在一个既有测试上失败,而该测试固定的正是本 PR 所声称的"保持 live 行为";且无论如何在这样规模的改动下都需要维护者介入。

独立来看,我对这个问题的方案与本 PR 几乎一致:封闭的、版本化的文档类型加逐 kind 白名单、永不序列化 raw payload 的单向投影器、复用现有 canonical 投影、nonce-CSP 模板且零远程资源、所有宿主收敛到同一渲染器。实现达到甚至超过了这一方案,防御层次(禁止字段扫描、第二轮完整复验、assertNever 穷尽、构建期拒绝内联脚本序列)比我预想的更好。这不是为了凑量,而是逐步遵循了维护者已合并的设计文档。

但有两点使其无法干净通过:

  • 红的 CI 恰好打破了核心声明。 失败的测试在 main 上原样存在,覆盖的正是本 PR 抽取并重新定义类型的 compact-mode 分组。如果抽取真的保持行为不变,该测试应当通过。要么抽取悄悄改变了什么(我的判断:很可能),要么测试是 flaky(请用重跑证明)。在此之前,"保持 interactive/readonly 语义"只是声明而非证明——对安全边界类 PR,兼容性声明与边界本身同等重要。
  • 规模属于人类判断而非门禁。 约 6,100 行生产逻辑横跨 7 个包,共享 SDK 预览路径的语义发生了变化(segmentId 合并规则、argsSummary 脱敏),CLI 的构建/运行时足迹即使在 --cli-only 下也在增长。读起来没有明显错误,但在这个规模下"读起来对"不是标准——Stage 0 规则把它划入维护者范围,我据此压低置信度而不是凭借尚可的阅读印象放行。

结论:就红的 CI 请求修改(修复或证明是 flake),其余交由 @yiliang114 定夺——您合并了本 PR 所基于的 prevalidation 阶段(#9388);值得您关注的是 SDK 预览语义的范围,以及 MR2A 应整体合入还是在 schema/投影器接缝处拆分。@water-in-stone 边界本身做得很好;阻塞点是窄而具体的。

Qwen Code · qwen3.8-max

Reviewed at 5f9470b974c2e60e33f0d3278566adf7a69669f1 · 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.

Requesting changes on one concrete blocker: the ubuntu CI lane is red on this head — "MessageList — compact mode > keeps a folded single-agent summary separate from an approving agent" fails in components/MessageList.dom.test.tsx (expected the msg-summary-t1 node to render, got null). That test is unchanged from main, while the grouping logic it pins is exactly what this PR extracted and retyped. Please fix it or demonstrate it is a flake with a re-run.

Everything else is in my review comments above — the security boundary reads well; the open items for the maintainer are the SDK preview-semantics scope and the MR2A size/split question. See the stage comments for details. 🙏

Preserve synthetic compact summary rows when parallel-agent grouping runs
after compact tool-group aggregation.

- Exclude summary rows from foreground agent-only grouping
- Exclude summary rows from background agent-only grouping
@water-in-stone

Copy link
Copy Markdown
Collaborator Author

Addressed the concrete compact-mode blocker from the change-request review and the Stage 2/3 triage comments in 937c45a437.

The extraction had dropped the existing summary-* exclusion from both foreground and background agent-only grouping. The fix restores that boundary so a synthetic compact summary remains separate from an agent that owns a pending approval.

Verification on the updated head:

  • Pre-fix focused regression: failed deterministically 3/3 times
  • Post-fix focused regression: 1/1 passed
  • Complete MessageList.dom.test.tsx: 136/136 passed
  • MessageList focused files: 278/278 passed
  • Full Web Shell suite: 4277/4277 passed
  • Repository build and typecheck: passed
  • Prettier, ESLint, and git diff --check: passed

The SDK preview-semantics scope and MR2A split remain maintainer decisions, as noted in the review; this fix is intentionally limited to the concrete behavior regression.

@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.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": did not execute integration-tests/chat-transcript-contract.test.ts to green — the worktree has no built packages/sdk-typescript/dist (the vitest aliases resolve….

Not reviewed: reverse audit — stopped before round 3 by the review time budget.

Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.

中文说明

仅完成部分审查,审查缺口已披露。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 1)"did not execute integration-tests/chat-transcript-contract.test.ts to green — the worktree has no built packages/sdk-typescript/dist (the vitest aliases resolve…

未审查:反向审计——评审时间预算不足,未能开始第 3 轮。

未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。

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

Comment thread packages/cli/src/ui/utils/export/export-transcript-document.ts Outdated
Comment thread packages/cli/src/ui/utils/export/export-transcript-document.ts
Comment thread packages/cli/src/ui/utils/export/markdown-document-policy.ts Outdated
Comment thread packages/cli/src/ui/utils/export/export-transcript-document.ts Outdated
Comment thread packages/cli/src/ui/utils/export/export-transcript-document.ts Outdated
Comment thread packages/web-shell/client/components/messages/PlanMessage.tsx
Comment thread packages/web-templates/src/export-html/src/document-main.tsx
Comment thread packages/web-templates/src/export-html/src/document-main.tsx
Comment thread packages/web-templates/src/export-html/build.mjs
Comment thread scripts/build.js
heyang.why added 5 commits August 27, 2026 14:19
# Conflicts:
#	packages/acp-bridge/src/transcript-replay.ts
#	packages/web-shell/client/components/MessageList.tsx
#	packages/web-shell/client/components/messages/ToolGroup.test.tsx
#	packages/web-shell/client/components/messages/tools/SubAgentPanel.tsx
qwen-code-ci-bot pushed a commit that referenced this pull request Aug 27, 2026
@water-in-stone

Copy link
Copy Markdown
Collaborator Author

Follow-up on the change-request review, Stage 2 triage, and Stage 3 triage on current PR head ef5d30fac1.

All 33 inline review threads now have an author reply and are resolved:

  • 31 findings were addressed with focused production or regression-test changes, including the export redaction/markdown/resource-budget boundary, typed preview correctness, background-agent terminal state, document renderer error handling, interactive Web Shell compatibility, and build-path scoping.
  • R1-14 and R1-15 were answered with an explicit boundary decision rather than represented as code fixes. The Chromium document gate remains a dedicated Playwright integration gate (4/4), and the no-access-key projection check remains an isolated integration boundary (151/151, with the focused contract gate 3/3). They were not added as ordinary workspace unit-test scripts because doing so would weaken their runtime/dependency isolation.
  • The earlier compact-mode regression remains fixed: folded synthetic summaries stay separate from approving agents.

Validation on this head:

  • npm run build: passed
  • npm run typecheck: passed
  • CLI export tests: 48/48
  • SDK daemon UI tests: 352/352
  • Web Shell focused/relevant tests: 413/413
  • VS Code export tests: 9/9
  • transcript contract gate: 3/3
  • Chromium document gate: 4/4
  • isolated no-access-key integration suite: 151/151 across 13 files

I left the historical CHANGES_REQUESTED reviews intact; they were not dismissed. The inline concerns have been replied to and resolved against the current published head.

@water-in-stone

Copy link
Copy Markdown
Collaborator Author

Final main synchronization update: PR head is now df02ec37e4, a normal merge of QwenLM/qwen-code main at d526612e92 on top of the reviewed/fixed head ef5d30fac1. The merge introduced no additional conflict resolution or PR-specific production changes.

I reran the relevant validation after this merge: repository build and typecheck passed; CLI export 54/54, SDK transcript/UI 391/391, Web Shell relevant tests 686/686 (including the compact-mode regression), VS Code export 9/9, transcript contract 3/3, Chromium document gate 4/4, and the isolated no-access-key integration gate 151/151 across 13 files all passed.

All 33 inline review threads remain replied to and resolved. The historical CHANGES_REQUESTED reviews remain intact and were not dismissed.

@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.

Partially reviewed — gaps disclosed.

Not explored to full depth (tool budget reached): chunk 23: none — no checks were cut short.; chunk 15: executing packages/cli/src/ui/utils/export/export-transcript-document.test.ts to confirm green at HEAD — the review worktree has no node_modules /dist and a ….

Not reviewed: reverse audit — stopped before round 3 by the review time budget.

Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.

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

  • integration-tests/chat-transcript-contract.test.ts:225 — [review] Schema-closure lock misses the toolResultPreview union (mutation stays green)
  • integration-tests/chat-transcript-contract.test.ts:91 — [review] Fixture-tree lock is fail-open for symlink entries
  • docs/design/web-shell/chat-transcript-contract-prevalidation.md:690 — [review] Six bare MR2 references orphan gate obligations after the MR2A/MR2B split
  • integration-tests/chat-transcript-document.test.ts:6 — [review] Browser gate imports playwright as a phantom dependency (webui hoist only)
  • packages/web-shell/client/adapters/parallelAgentGrouping.ts:30 — [review] Summary-row exclusion hardcodes summary- instead of the shared summaryRunId predicate
  • packages/acp-bridge/src/transcript-replay.ts:1262 — [review] Unreachable shell_output/tool_output lane branch in transcriptSegmentLane
  • packages/web-templates/src/export-html/build.mjs:142 — [review] Post-transform <script guard is dead code; misleading safety model
  • packages/web-shell/client/components/MessageList.tsx:2093 — [review] data-source-block-ids written on every row but read nowhere; includeSourceIdentity hard-wired for it
  • packages/web-shell/client/adapters/transcriptToMessages.ts:480 — [review] sourceBlockIds missing on three block-backed pushes (mid_turn/vision_bridge)
中文说明

仅完成部分审查,审查缺口已披露。

未探索到全部深度(达到工具调用预算):chunk 23:none — no checks were cut short.;chunk 15:executing packages/cli/src/ui/utils/export/export-transcript-document.test.ts to confirm green at HEAD — the review worktree has no node_modules /dist and a …

未审查:反向审计——评审时间预算不足,未能开始第 3 轮。

未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。

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

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

Comment on lines +1945 to +1947
source.slice(0, offset + prefix.length).endsWith('[home]')
? match
: prefix + '[home]',

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.

[Critical] R1-1: (fix-induced) The home-path redaction boundary is still bypassable at this head — the replacement mechanism introduced by the round-1 fix has three executed bypass entrances, so the round-1 class finding stands. (1) Leading-slash/dot-segment spellings defeat the boundary class: //home/alice/secret.txt, /./home/alice/..., //Users/bob/Library/Keychains export verbatim with complete:true (the boundary class excludes / and . before home|Users; the redundant-segment cleanup only runs after home/). (2) Input-borne [home] grants immunity: text containing [home]/home/alice/.ssh/id_rsa suppresses both the redactor and the residual detector via the endsWith('[home]') guards, so a raw home path reaches the shareable export while home_path_forbidden passes — while the percent-encoded twin fails closed. (3) Nested home paths leak in the single replace pass: /home/alice/home/bob/secret.txt exports as [home]/home/bob/secret.txt (substituted output is never rescanned, and the detector exemption clears the residue) — and export-transcript-document.test.ts:201 pins the leaking shape. Enumerating more spellings will not converge: the entrance space of path spellings in arbitrary text has no last corner.

Witness — probes at this head:

CONTROL in: "leak /home/alice/.ssh/id_rsa end" -> "leak [home]/.ssh/id_rsa end"
BYPASS in: "leak [home]/home/alice/.ssh/id_rsa end" -> unchanged, complete:true, diagnostics:[]
in: "nested /home/alice/home/bob/secret.txt" -> "nested [home]/home/bob/secret.txt", complete:true
in: "//home/alice/secret.txt", "/./home/alice/...", "//Users/bob/Library/Keychains" -> exported verbatim, complete:true
ASYMMETRY in: "leak [home]%2Fhome%2Falice%2Fsecret end" -> throws home_path_forbidden (fails closed)

Suggested fix: match the sensitive structure instead of a grammar — extract path-like and percent-encoded tokens, normalize them (collapse //, resolve . segments, decode percent-encoding) and redact when the normalized token is prefixed by the actual home directory captured at export time; delete both endsWith('[home]') guards so the detector flags every raw /home|/Users/<seg> occurrence (detector must be a strict superset of the redactor); update the nested= expectation at export-transcript-document.test.ts:201, which pins the leaking shape.

Fix witness: add cases asserting [home]/home/alice/private.txt, //home/alice/x, and /home/alice/home/bob/secret.txt each export without /home/alice or /home/bob surviving, then remove the new redaction logic and confirm those tests go red.

中文说明

本 head 上主目录脱敏边界仍可被绕过——round 1 修复引入的新机制存在三个已实测的绕出入口,因此 round 1 的类级发现仍然成立。(1) 前导斜杠/点段拼写可绕过边界类://home/alice/secret.txt/./home/alice/...//Users/bob/Library/Keychains 都会原样导出且 complete:true(边界类在 home|Users 之前排除了 /.,冗余段清理只在 home/ 之后生效)。(2) 输入自带的 [home] 会获得豁免:包含 [home]/home/alice/.ssh/id_rsa 的文本通过 endsWith('[home]') 守卫同时压制了脱敏器与残留检测器,原始主目录路径进入可分享导出而 home_path_forbidden 放行——其百分号编码孪生形态却是快速失败(抛错)的。(3) 嵌套主目录路径在单次替换中泄漏:/home/alice/home/bob/secret.txt 导出为 [home]/home/bob/secret.txt(替换结果从不重扫描,且检测器的豁免清除了残留)——并且 export-transcript-document.test.ts:201 把泄漏形态固化成了断言。继续枚举更多拼写不会收敛:任意文本中路径拼写的入口空间没有最后一个角。

建议修复:改为匹配敏感结构本身而非语法——提取路径状与百分号编码的 token,做归一化(折叠 //、解析 . 段、解码百分号),当归一化 token 以导出时捕获的真实主目录为前缀时脱敏;删除两个 endsWith('[home]') 守卫,使检测器标记每一处原始 /home|/Users/<seg>(检测器必须是脱敏器的严格超集);更新 export-transcript-document.test.ts:201nested= 断言,它目前固化的是泄漏形态。

修复见证:新增用例断言 [home]/home/alice/private.txt//home/alice/x/home/alice/home/bob/secret.txt 导出后不含 /home/alice/home/bob,然后移除新脱敏逻辑并确认这些测试变红。

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

Comment on lines +301 to +302
const root = parseMarkdown(value);
if (!root) return MARKDOWN_COMPLEXITY_FALLBACK;

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.

[Critical] R1-6: (fix-induced) The complexity bound added for round-1 R1-6's re-parse hang has a silent path: transformRichMarkdownTasks hits the complexity fallback with no onComplexityLimit hook, while its sibling sanitizeMarkdownDocument reports via policy.onComplexityLimit?.() — so ExportBudget.text() cannot record truncated or the markdown_complexity_exceeded diagnostic. assertResourceBudgets accepts the fallback (it re-sanitizes cleanly), and the exported document silently loses an entire entry while claiming completeness — violating this module's own degraded/complete consistency invariant. Note the marker count can RISE between the two passes (each rejected autolink like <javascript:alert(1)> becomes [link omitted], 1 marker to 2), so a fence near the cap after sanitize can exceed it in the transform.

Witness — probe at this head:

in: code fence + ~2049 '[' markers (no navigable content)
out: entry replaced by '[markdown omitted: complexity limit exceeded]'
     complete:true, truncated:false, diagnostics:[]
control: identical marker load without the fence survives (complete:true)
flip: wiring onComplexityLimit -> complete:false, truncated:true, diagnostics:[markdown_complexity_exceeded]

Suggested fix: give transformRichMarkdownTasks a complexity callback invoked on the !root branch (or fold it into the policy object), and wire it in ExportBudget.text() to set truncated and add markdown_complexity_exceeded, mirroring the sanitize path.

Fix witness: in markdown-document-policy.test.ts, feed transformRichMarkdownTasks a value whose marker count exceeds the cap after sanitize and assert the complexity callback fires; removing the callback invocation must make it red.

中文说明

为 round 1 R1-6 的重解析挂起问题新增的复杂度边界存在一条静默路径:transformRichMarkdownTasks 命中复杂度回退时没有 onComplexityLimit 钩子,而其姊妹函数 sanitizeMarkdownDocument 会通过 policy.onComplexityLimit?.() 上报——因此 ExportBudget.text() 无法记录 truncatedmarkdown_complexity_exceeded 诊断。assertResourceBudgets 会接受该回退结果(它能被干净地重新脱敏),导出文档在声称完整的同时静默丢失整个条目——违反了本模块自身的 degraded/complete 一致性不变量。注意两次解析之间标记数可能上升(每个被拒绝的 autolink(如 <javascript:alert(1)>)变成 [link omitted],1 个标记变 2 个),因此 sanitize 后接近上限的围栏在 transform 阶段可能超限。

建议修复:为 transformRichMarkdownTasks 增加一个在 !root 分支调用的复杂度回调(或并入 policy 对象),并在 ExportBudget.text() 中接入,设置 truncated 并添加 markdown_complexity_exceeded,与 sanitize 路径保持一致。

修复见证:在 markdown-document-policy.test.ts 中,给 transformRichMarkdownTasks 输入 sanitize 后标记数超过上限的内容并断言复杂度回调被触发;移除该回调调用后测试必须变红。

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

Comment on lines +439 to +441
const images = block.images
? budget.array(block.images).flatMap((image) => {
const safe = budget.image(image);

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.

[Critical] R2-1: The user/assistant/thought branch of sanitizeBlock exports images but silently drops block.files (file attachments) — no schema field, no diagnostic, no markContentLoss() — so the V1 document loses user-visible attachment content while metadata.complete stays true, violating the pipeline's own invariant that every content loss surfaces via a diagnostic and/or the completeness flag. Every comparable loss in this module is tracked (image_type_rejected, tool_result_presentation_missing + markContentLoss, record_internal_excluded, debug_block_excluded); image-type attachment references even degrade to a visible '[Attachment is no longer available]' placeholder — file attachments alone vanish without a word.

Witness — probe at this head (real pipeline): a user record whose projection-level block carries files:[{name,mimeType,attachmentId}] exports as {"kind":"user","text":"please review the log","streaming":false} with fileTracePresentAnywhere:false, complete:true, diagnostics:[]; an image reference on the same path degrades to visible placeholder text.

Suggested fix: in this branch add a tracked loss when attachments exist (diagnostics.add('file_attachment_excluded', 'warning', block.files.length, true); budget.markContentLoss();), or extend ExportTextTranscriptBlockBaseV1 and the schema with a sanitized files array (budget.label-bounded name/attachmentId + mimeType) mirroring the images handling.

Fix witness: add a case feeding a user record whose systemPayload.attachmentReferences holds one resource entry and assert the exported block either carries the attachment or metadata.complete === false with the content-loss diagnostic; removing the added branch must make it red.

中文说明

sanitizeBlock 的 user/assistant/thought 分支会导出 images 却静默丢弃 block.files(文件附件)——没有 schema 字段、没有诊断、没有 markContentLoss()——因此 V1 文档丢失用户可见的附件内容而 metadata.complete 仍为 true,违反了本管线自身“任何内容损失都必须通过诊断和/或完整性标志体现”的不变量。本模块中所有可比的损失都有跟踪(image_type_rejectedtool_result_presentation_missing + markContentLossrecord_internal_excludeddebug_block_excluded);图片型附件引用甚至会退化为可见的 '[Attachment is no longer available]' 占位符——唯独文件附件无声消失。

建议修复:在该分支中,当存在附件时添加受跟踪的损失(diagnostics.add('file_attachment_excluded', 'warning', block.files.length, true); budget.markContentLoss();),或者为 ExportTextTranscriptBlockBaseV1 与 schema 增加脱敏后的 files 数组(经 budget.label 限制的 name/attachmentId + mimeType),参照 images 的处理方式。

修复见证:新增用例,输入 systemPayload.attachmentReferences 含一个 resource 条目的 user 记录,断言导出块要么携带附件、要么 metadata.complete === false 且带内容损失诊断;移除新增分支后测试必须变红。

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

Comment on lines +967 to +969
label(value: unknown, maxLength: number, redact = true): string {
const safe = safeLabel(value, maxLength);
if (safe !== value) this.markTruncated('label_sanitized');

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.

[Critical] R2-2: label() truncates to maxLength BEFORE running redactHomePaths, but the file: home-URL redaction can GROW the string (file:/home/a, 12 chars, becomes file://[home], 13) — so a capped identifier can leave label() at maxLength + 1, and the schema maxLength validation then throws schema_validation_failed, aborting the whole export instead of degrading. safeMetadataLabelbudget.plainText has the same redact-after-check ordering, and the metadata loop re-checks at the end. Reachable fields include user/model-controlled text (mcp serverId cap 128, ask_user_question header cap 200, metadata model/gitBranch/channel), not contrived inputs.

Witness — probes at this head:

mcp serverId 'x'.repeat(116)+'file:/home/a' (128 chars) -> throws schema_validation_failed
control same length ending 'file:/home/' (no growth)   -> document created OK
metadata.model 'm'.repeat(188)+'file:/home/a' (200)    -> throws schema_validation_failed

Suggested fix: redact before capping — in label(), apply redactHomePaths to the raw value and run safeLabel(..., maxLength) on the redacted result (or re-cap after redaction); in safeMetadataLabel, evaluate length after redaction.

Fix witness: add a regression test where an ask_user_question header (or mcp serverId) at the cap containing file:/home/a within the first cap chars yields a document (with truncated: true), not a throw; removing the re-cap makes it red.

中文说明

label() 在运行 redactHomePaths 之前就把字符串截断到 maxLength,但 file: 主目录 URL 脱敏可能使字符串变长(file:/home/a 12 字符变为 file://[home] 13 字符)——因此一个恰好达到上限的标识符可能以 maxLength + 1 的长度离开 label(),随后 schema 的 maxLength 校验抛出 schema_validation_failed,使整个导出中止而非降级。safeMetadataLabelbudget.plainText 同样是“先检查后脱敏”的顺序,且 metadata 循环末尾会再次检查。可达字段包括用户/模型可控文本(mcp serverId 上限 128、ask_user_question header 上限 200、metadata model/gitBranch/channel),并非构造输入。

建议修复:先脱敏再限长——在 label() 中对原始值应用 redactHomePaths,再对脱敏结果运行 safeLabel(..., maxLength)(或在脱敏后重新限长);在 safeMetadataLabel 中于脱敏之后再评估长度。

修复见证:新增回归测试:长度恰好达到上限且前若干个字符内含 file:/home/a 的 ask_user_question header(或 mcp serverId)必须产出文档(带 truncated: true)而不是抛错;移除重新限长逻辑后测试必须变红。

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

Comment on lines +65 to +67
let markers = 0;
for (const character of value) {
if (character !== '[' && character !== ']' && character !== '>') continue;

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.

[Critical] R2-3: The pre-parse complexity veto only counts [/]/> markers, but the super-linear remark-parse costs it exists to bound come from constructs containing NONE of those markers — emphasis-delimiter soup and indentation-nested lists. The cost is paid inside parseMarkdown before any AST budget runs (ExportBudget.text() sanitizes before applyTextBudget), and assertResourceBudgets clamps to maxTextBytes but still parses. One transcript message carrying a single ], @, or <> pair (routine in content full of code/log output) passes the navigability gate and stalls an export for a minute; entries compound (visible-text budget is 8 MB), and text() re-parses via transformRichMarkdownTasks when fences are present. This is the under-count twin of the marker over-deletion finding (R2-9); raising the marker cap does not close it.

Witness — wall times through createExportTranscriptDocumentV1 at this head:

'a*'.repeat(50_000) (no marker, control):       125.7 ms
'a*'.repeat(10_000)+']' (20 KB):              5,457 ms (parsed, exported)
'a*'.repeat(20_000)+']' (40 KB):              8,847 ms (AST budget rejects only AFTER parse)
'a*'.repeat(50_000)+']' (100 KB):            56,398 ms  (2.5x bytes -> 6.4x time)
nested list 324,801 bytes, 0 counted markers: 4,609 ms

Suggested fix: extend the veto in parseMarkdown with cheap structural checks for the marker-less super-linear shapes — veto when any line's leading indentation exceeds 2 * MAX_MARKDOWN_AST_DEPTH spaces, and cap */_ emphasis-delimiter counts (or delimiter-dense bytes) the same way markers are capped; alternatively cap input bytes before parsing and route the excess through onComplexityLimit.

Fix witness: in markdown-document-policy.test.ts, assert sanitizeMarkdownDocument('a*'.repeat(50_000) + ']') returns the complexity fallback under a wall-clock bound, plus a zero-marker deeply-indented list case; removing the added veto makes them red by timeout.

中文说明

解析前复杂度否决只统计 [/]/> 标记,但它要约束的 remark-parse 超线性开销恰恰来自不含任何这些标记的结构——强调分隔符汤与缩进嵌套列表。开销发生在 parseMarkdown 内部、任何 AST 预算生效之前(ExportBudget.text() 先脱敏后预算),assertResourceBudgets 虽然按 maxTextBytes 限流但仍会解析。只要一条转录消息里带一个 ]@<> 对(在充满代码/日志输出的内容中很常见)就能通过可导航性闸门,使一次导出停滞约一分钟;条目可叠加(可见文本预算 8 MB),且有围栏时 text() 还会经 transformRichMarkdownTasks 再次解析。这是“标记过量删除”发现(R2-6)的孪生问题(计数不足方向);提高标记上限并不能堵住它。

建议修复:在 parseMarkdown 的否决中为无标记的超线性形态增加廉价结构检查——任一行前导缩进超过 2 * MAX_MARKDOWN_AST_DEPTH 个空格即否决,并像标记一样对 */_ 强调分隔符数量(或分隔符密集字节数)设上限;或者在解析前对输入字节数设上限,超出部分走 onComplexityLimit

修复见证:在 markdown-document-policy.test.ts 中断言 sanitizeMarkdownDocument('a*'.repeat(50_000) + ']') 在墙钟时限内返回复杂度回退,另加一个零标记深缩进列表用例;移除新增否决后它们会因超时变红。

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

Comment on lines +160 to +161
const previewNewText = tool.args?.newText;
if (typeof previewNewText === 'string') {

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.

[Critical] R2-6: The args-based diff preview — the ONLY diff source on the document/export projection (content is forced undefined there and rawOutput is just the result-preview text) — silently skips delete-only edits. getFirstString rejects empty strings, so detectFileDiff on new_string: '' emits {kind:'file_diff', path, oldText} with NO newText; this fallback requires newText to be a string, returns '', and the exported HTML shows a completed deletion with its one-line result but no diff — while every replacement edit (oldText+newText) and every create edit (old_string: ''buildUnifiedDiff('', newText) works) renders a diff. Silent content loss in an artifact whose metadata.complete is true.

Witness — probe end-to-end at this head:

UPSTREAM SDK: edit args {file_path, old_string:'deleted line', new_string:''}
  -> preview {kind:'file_diff', oldText:'deleted line'} (no newText; create-edit control carries newText)
DOCUMENT PROJECTION: delete-only preview -> args {path, oldText} -> extractDiff="" (completed deletion, no diff)
CONTROL create edit -> extractDiff="-\n+added line"
FLIP accepting oldText-only previews -> extractDiff="-deleted line\n+"

Suggested fix: accept oldText-only previews — branch on typeof previewNewText === 'string' || typeof previewOldText === 'string' and call buildUnifiedDiff with '' for the missing side; alternatively preserve newText: '' in detectFileDiff when the edit shape is detected.

Fix witness: a ToolGroup.test.tsx safe-projection case whose block preview is {kind:'file_diff', path:'document.ts', oldText:'old content'} (no newText) asserting extractDiff(tool) contains -old content; removing the oldText-only branch makes it red.

中文说明

基于 args 的 diff 预览是 document/export 投影上唯一的 diff 来源(那里 content 被强制为 undefinedrawOutput 只是结果预览文本),但它静默跳过纯删除编辑。getFirstString 拒绝空字符串,因此 detectFileDiffnew_string: '' 只产出 {kind:'file_diff', path, oldText} 而没有 newText;此回退分支要求 newText 必须是字符串,于是返回 '',导出的 HTML 对已完成的删除只显示一行结果而没有 diff——而所有替换编辑(oldText+newText)和所有创建编辑(old_string: ''buildUnifiedDiff('', newText) 可工作)都会渲染 diff。这是在 metadata.completetrue 的产物中静默丢失内容。

建议修复:接受仅含 oldText 的预览——以 typeof previewNewText === 'string' || typeof previewOldText === 'string' 为条件,对缺失的一侧以 '' 调用 buildUnifiedDiff;或者在 detectFileDiff 检测到编辑形态时保留 newText: ''

修复见证:新增 ToolGroup.test.tsx safe 投影用例,块预览为 {kind:'file_diff', path:'document.ts', oldText:'old content'}(无 newText),断言 extractDiff(tool) 包含 -old content;移除仅 oldText 分支后测试必须变红。

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

Comment on lines +10 to +12
export {
DOCUMENT_HTML_TEMPLATE as EXPORT_TRANSCRIPT_HTML_TEMPLATE,
EXPORT_TRANSCRIPT_RENDERER_LIMITS,

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.

[Critical] R2-7: This PR inlines the ~20 MB document template into the shipped npm bundle (these exports pull generated/exportTranscriptDocumentTemplate.ts into the CLI import graph via formatters/html.ts; esbuild emits it as a ~20.7 MB chunk that prepare-package.js ships wholesale) but leaves DEFAULT_MAX_NPM_PACKAGE_UNPACKED_BYTES = 96 MiB unchanged — so npm run prepare:package throws. No PR lane runs prepare:package: only release.yml's publish job (nightly cron, Tuesday preview cron, workflow_dispatch), desktop-shell prepare-runtime.js, the vscode companion prepackage.js, and the tag-triggered Docker build do. The PR merges fully green and the first red lands on the next release lane.

Witness — measured in this worktree's build:

BASE: node scripts/prepare-package.js -> Error: Prepared package unpacked size 117432625 bytes exceeds 100663296 bytes
FLIP: stub out the 20,760,649-byte template chunk (verified to contain the PR's 'transcript-document' envelope) -> gate passes (~92.2 MiB)

scripts/tests/package-assets.test.js:933 pins the 100663296 default.

Suggested fix: bump DEFAULT_MAX_NPM_PACKAGE_UNPACKED_BYTES to cover the new headroom (e.g. 128 MiB) together with the pinning test, or reduce shipped bytes if a slimmer template representation is feasible — decide it in this PR, since the failure otherwise surfaces only on the release lane post-merge.

Fix witness: scripts/tests/package-assets.test.js — 'enforces a 96 MiB default unpacked size budget' asserts the exact exceeds 100663296 bytes message and must move in lockstep with any cap bump; 'fails packaging when prepared dist exceeds the unpacked size budget' pins the gate itself.

中文说明

本 PR 把约 20 MB 的文档模板内联进了发布的 npm 包(这些导出经由 formatters/html.tsgenerated/exportTranscriptDocumentTemplate.ts 拉入 CLI 导入图;esbuild 将其产出为约 20.7 MB 的 chunk,prepare-package.js 会整体打包发布),但 DEFAULT_MAX_NPM_PACKAGE_UNPACKED_BYTES = 96 MiB 未变——因此 npm run prepare:package 会抛错。没有任何 PR 门禁会运行 prepare:package:只有 release.yml 的 publish job(夜间定时、周二预览定时、workflow_dispatch)、desktop-shell 的 prepare-runtime.js、vscode companion 的 prepackage.js 以及 tag 触发的 Docker 构建会运行。PR 会以全绿合入,第一个红会出现在下一个 release 门禁。

建议修复:提高 DEFAULT_MAX_NPM_PACKAGE_UNPACKED_BYTES 以覆盖新增体积(例如 128 MiB)并同步更新锁定测试;或者若有更精简的模板表示则减少发布字节——请在本 PR 内决定,否则该失败只会在合入后的 release 门禁暴露。

修复见证:scripts/tests/package-assets.test.js 中 'enforces a 96 MiB default unpacked size budget' 断言了精确的 exceeds 100663296 bytes 文案,任何上限调整都必须同步;'fails packaging when prepared dist exceeds the unpacked size budget' 锁定门禁本身。

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

Comment thread .github/workflows/e2e.yml
Comment on lines +295 to +296
- name: 'Run transcript document browser gate'
run: 'npx vitest run --root ./integration-tests ./chat-transcript-document.test.ts --retry=0'

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.

[Critical] R2-8: The web-shell-browser-regression job this gate step was added to keeps its single npx playwright install --with-deps chromium, which installs at most ONE chromium revision — while the job now needs TWO: this test imports root-hoisted playwright@1.58.2 (chromium rev 1208), but npx playwright resolves to .bin/playwright = @playwright/test's CLI (1.61.1, chromium rev 1228). The job runs only on schedule/workflow_dispatch, never on PR checks — so this PR's green checks give no evidence about it, and the first red lands on main after merge. This PR already fixed the identical shape in ci.yml's web_shell_e2e_smoke with a dual-tree install.

Witness — probes on this tree:

node_modules/.bin/playwright -> ../@playwright/test/cli.js (1.61.1 -> chromium rev 1228)
hoisted playwright 1.58.2 browsers.json -> chromium rev 1208
npx playwright install --dry-run chromium                      -> Install location .../chromium-1228
node node_modules/playwright/cli.js install --dry-run chromium -> .../chromium-1208
with only 1228 present: LAUNCH FAILED: browserType.launch: Executable doesn't exist at .../ms-playwright/chromium_headless_shell-1208/...
after the dual install: LAUNCH OK

Suggested fix: mirror ci.yml's dual-tree install in this job:

- name: 'Install Playwright Chromium'
  run: |-
    node node_modules/playwright/cli.js install --with-deps chromium
    nested_cli='node_modules/@playwright/test/node_modules/playwright/cli.js'
    if [ -f "${nested_cli}" ]; then
      node "${nested_cli}" install chromium
    fi

Fix witness: N/A — the witness is this job's own gate step on the nightly lane (red without the fix); no in-repo unit test can pin a runner-side browser install.

中文说明

该门禁步骤被加入的 web-shell-browser-regression job 仍然只有单条 npx playwright install --with-deps chromium,最多安装一个 chromium 版本——而该 job 现在需要两个:本测试导入根目录提升的 playwright@1.58.2(chromium rev 1208),但 npx playwright 解析到 .bin/playwright = @playwright/test 的 CLI(1.61.1,chromium rev 1228)。该 job 仅在 schedule/workflow_dispatch 运行、从不在 PR 检查中运行——因此本 PR 的绿色检查对它没有任何证据,第一个红会在合入后落在 main 上。本 PR 已经在 ci.yml 的 web_shell_e2e_smoke 中用双树安装修复了同样的形态。

建议修复:在该 job 中镜像 ci.yml 的双树安装(见上方 YAML)。

修复见证:N/A——见证就是该 job 自身在夜间门禁上的门禁步骤(不修复则红);仓库内没有单元测试能锁定 runner 侧的浏览器安装。

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

Comment on lines +65 to +67
let markers = 0;
for (const character of value) {
if (character !== '[' && character !== ']' && character !== '>') continue;

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-9: The pre-parse marker veto counts every [, ], > anywhere in the raw source — including inside code fences and plain prose — so it does not correlate with parse cost and over-deletes benign content: a ~15 KB flat-paragraph log of bracketed tokens is dropped from exports entirely (tracked loss with diagnostic + truncated, but over-deletion). Boundary measured at this head: 341 log-style lines (2046 markers) survive with complete:true; 342 lines (2052 markers) are replaced by the complexity fallback. Transcripts quoting logs, JSON arrays, or shell output (> prompts, =>, redirects) hit this routinely — reachable from exactly the fields coding-assistant transcripts are full of.

Witness — probes at this head: 342 lines shaped [2026-01-01 12:00:NN] [INFO] [worker-N] ok → entry replaced, complete:false, diagnostics [markdown_complexity_exceeded]; 341 lines → survive, complete:true.

Suggested fix: make the veto track nesting-relevant markers instead of raw character counts (max leading-blockquote > depth per line and bracket nesting depth, or exclude markers inside fenced code), or raise MAX_MARKDOWN_SOURCE_MARKERS substantially — keeping the AST node/depth budgets and the parse try/catch as the authoritative fail-closed gates.

Fix witness: assert that an entry with 2048+ non-nesting markers (bracketed log lines, or brackets inside a fenced code block) is returned intact rather than replaced with the fallback; goes red while the naive character count remains.

中文说明

解析前的标记否决统计原始文本中任意位置的每一个 []>——包括代码围栏内部和普通行文——因此它与解析开销并不相关,会过量删除良性内容:一段约 15 KB、由带括号 token 组成的扁平段落日志会被整个从导出中丢弃(有诊断 + truncated 的受跟踪损失,但属于过量删除)。在本 head 实测边界:341 行日志形态(2046 个标记)存活且 complete:true;342 行(2052 个标记)被替换为复杂度回退。引用日志、JSON 数组或 shell 输出(> 提示符、=>、重定向)的转录会经常命中——可达面正是编程助手转录中最常见的字段。

建议修复:让否决跟踪与嵌套相关的标记而非原始字符计数(每行前导块引用 > 的最大深度与括号嵌套深度,或排除围栏代码内的标记),或大幅提高 MAX_MARKDOWN_SOURCE_MARKERS——保留 AST 节点/深度预算与解析 try/catch 作为权威的快速失败闸门。

修复见证:断言含 2048+ 个非嵌套标记的条目(日志括号行或围栏代码内的括号)原样返回而非被替换为回退文案;保留朴素字符计数时该断言为红。

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

Comment thread .github/workflows/ci.yml
Comment on lines 826 to 827
- name: 'Install Playwright Chromium (self-hosted)'
if: "${{ runner.environment == 'self-hosted' }}"

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.

[Critical] R2-10: The rewrite of this job's Playwright install steps (npx playwright install chromium → the dual-tree node node_modules/playwright/cli.js install form) breaks the repository's CI-wiring contract test, which pins the old shape: scripts/tests/no-ak-integration-ci.test.js 'does not install Linux packages on self-hosted Playwright runners' expects run: 'npx playwright install chromium' in web_shell_e2e_smoke. The ubuntu unit-test CI job is red on this very test at the reviewed head, and the failure reproduces deterministically in a clean checkout. The workflow change itself is intentional (round-1 R1-12 discussed it); the pin was never updated with it. Merge-base main still carries the pinned single-install form, so the divergence is this PR's.

Witness — executed:

npx vitest run --project scripts scripts/tests/no-ak-integration-ci.test.js (at this head)
  x 'does not install Linux packages on self-hosted Playwright runners' (AssertionError at :656)
  Test Files 1 failed | 65 passed (66); Tests 1 failed | 1779 passed
CI at the same head: Test (ubuntu-latest, Node 22.x) failed with the identical assertion

Suggested fix: update the pin to the new dual-tree shape — assert the node node_modules/playwright/cli.js install chromium step plus the nested-CLI fallback, keeping the self-hosted no---with-deps assertion — so the contract test locks the form the workflow now uses.

Fix witness: the updated assertion itself — reverting this job's self-hosted step to the old npx form must turn it red.

中文说明

该 job 的 Playwright 安装步骤重写(npx playwright install chromium → 双树 node node_modules/playwright/cli.js install 形态)破坏了仓库的 CI 接线契约测试,该测试固化的是旧形态:scripts/tests/no-ak-integration-ci.test.js 的 'does not install Linux packages on self-hosted Playwright runners' 期望 web_shell_e2e_smoke 中有 run: 'npx playwright install chromium'。在被审查的 head 上,ubuntu 单元测试 CI job 正因这个测试而红,且在干净检出中可确定性复现。工作流改动本身是有意的(round 1 R1-12 讨论过),但固化断言从未随之更新。merge-base main 仍是被固化的单安装形态,因此分叉来自本 PR。

建议修复:把断言更新为新的双树形态——断言 node node_modules/playwright/cli.js install chromium 步骤及嵌套 CLI 回退,保留 self-hosted 不带 --with-deps 的断言——使契约测试锁定工作流当前使用的形态。

修复见证:更新后的断言本身——把该 job 的 self-hosted 步骤还原为旧 npx 形态必须使其变红。

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants