feat(transcript): add cross-host contract prevalidation - #9355
feat(transcript): add cross-host contract prevalidation#9355water-in-stone wants to merge 3 commits into
Conversation
Establish a shared read-only transcript boundary for Web Shell, VS Code, Desktop, and future HTML export without changing interactive adapter semantics. - Add versioned fixtures, schemas, capability gates, and identity probes - Preserve raw runtime payloads while adding safe document projections - Add source-stable ACP identity and read-only VS Code ID projection - Add document rendering, export allowlists, and network guards - Cover direct-daemon, ACP, HTML export, and Web Shell regressions Note: Production VS Code and HTML renderer migrations remain follow-ups.
|
Thanks for the PR — this is a big one, so a few honest gate observations before code review.
Flagging these for maintainer attention, and moving on to code review. 🔍 中文说明感谢贡献!这个 PR 体量很大,先说几点门禁阶段的观察。
以上提请 maintainer 关注,同时进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent baseline first: for "repeatable evidence that the existing transcript path preserves semantics and identity across hosts", I'd have landed a thin fixture-based equivalence suite against the path we already ship (fixtures + replay/render assertions, ~1-2k lines including tests), verified identity with the IDs the protocol already carries, and left the export hardening to the migration PR that needs it — no wire changes, no new production surface. What this PR does instead is broader. The good parts, verified statically: the default-path-preservation claim holds up in the diff. Findings worth a maintainer's attention:
Not verified: runtime behavior of any of the above — no CI has run on this commit (below), so the "default behavior unchanged" and "segmentId is inert" claims rest on static reading alone. The author's reported results (macOS only, 644 CLI tests + 7 integration tests passing) are the author's claim, not evidence this review can cite. Files changed (30 of 77 shown)
…and 47 more files — unit tests for the modules above, small CSS-module tweaks for the new mode, and fixture case files. Test evidence (this PR's own CI)PR CI has not run on this commit at all: there are zero
Sandboxed verification would settle the behavioural claims: 中文说明代码审查独立基线:要证明"现有 transcript 路径跨宿主保持语义与身份一致",我会先落地一套精简的 fixture 等价性测试(fixture + replay/render 断言,含测试约 1-2k 行),用协议已有的 ID 验证身份,把导出加固留给真正需要它的迁移 PR——不改协议、不新增生产面。 本 PR 做得更多。静态验证过的好消息:默认路径保持不变的声明在 diff 中成立。 提请 maintainer 关注的发现:
未验证:以上所有点的运行时行为——本 commit 没有任何 CI 运行,因此"默认行为不变"与"segmentId 无副作用"目前只有静态阅读支撑。作者自报的结果(仅 macOS,644 个 CLI 测试 + 7 个集成测试通过)是作者声明,不是本审查可引用的证据。 测试证据(本 PR 自己的 CI)本 commit 完全没有跑过 PR CI:head SHA 上没有任何 沙箱验证可以落定行为性声明: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — competent, careful engineering, but the Stage 0 size escalation caps the verdict at a maintainer decision, and there are real open questions underneath the cap anyway. Stepping back: the code I reviewed is mostly good. Defaults are preserved, the document-mode gating is consistent across a dozen components, budgets and caps fail closed, and the author clearly knows this codebase — their merged track record in web-shell and core shows. Nobody should read this as sloppy work. But the gate's job is to ask whether this should ship in this shape, and here I have genuine reservations:
So this is not a reject — it's a "this needs a human decision before it goes further". ⏸️ Deferring to @wenshao @pomelo-nwu — escalating per the core-breadth size rule (5,722 production lines across five packages;
The two concrete code findings from Stage 2 (test-only consumers; ungated 中文说明置信度:3/5 —— 工程上是胜任且细致的,但 Stage 0 的规模升级把结论封顶在"由 maintainer 决定",而且封顶之下本来也有真实的疑问。 退一步看:我审过的代码大部分是好的。默认行为被保留,document 模式门控在十几个组件间保持一致,预算与上限都是 fail-closed,作者显然了解这个代码库——他们在 web-shell 和 core 的合并记录可以证明。不应把这里的评价读成"工作粗糙"。 但门禁的职责是问"这个应不应该以这种形态合入",在这一点上我有真实的保留:
所以这不是拒绝——而是"在继续之前需要一个人类决定"。 ⏸️ 移交 @wenshao @pomelo-nwu —— 按核心广度规模规则升级(5,722 行生产代码横跨五个包;
无论方向结论如何,Stage 2 的两个具体代码发现(仅测试消费者;未门控的 — Qwen Code · qwen3.8-max Reviewed at |
1287487 to
b6e691c
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 17 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): chunk 8: run integration-tests/chat-transcript-contract.test.ts end-to-end (worktree has no node_modules/build; install+build+bundle exceeds this chunk's tool budget — a….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查,审查缺口已披露。
未探索到全部深度(达到工具调用预算):chunk 8:run integration-tests/chat-transcript-contract.test.ts end-to-end (worktree has no node_modules/build; install+build+bundle exceeds this chunk's tool budget — a…。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const params: SessionNotification = { | ||
| sessionId: this.sessionId, | ||
| update: projectedUpdate, | ||
| update: this.transcriptUpdateIdentity.project( |
There was a problem hiding this comment.
[Critical] The PR breaks the previously-green packages/cli suite: sendUpdate now stamps _meta.qwenTranscript.segmentId onto every update — including replayed history — but history-replayer.test.ts's exact-match expectations were not updated. — Failure scenario: any npm test / CI run of packages/cli: HistoryReplayer replays records through Session.sendUpdate, the projector injects segmentId, and 10 toHaveBeenCalledWith assertions fail on the extra field — the workspace test command exits non-zero in every environment (the failing Test (ubuntu-latest, Node 22.x) CI job is consistent with this).
Witness (executed):
PR tree: Tests 10 failed | 37 passed (47); merge base (121f247c0): 47/47 passed — measured on both trees
Suggested fix: Update history-replayer.test.ts expectations to include the projected identity metadata (as was done for the Session.test.ts refresh), or skip projection on the history-replay path if segment identity is meant to be live-session-only.
中文说明
sendUpdate 现在对每条 update(包括回放的历史记录)都注入 _meta.qwenTranscript.segmentId,导致 packages/cli 既有绿套件 history-replayer.test.ts 挂掉 10/47(merge base 上 47/47 全绿,双树实测)。The PR breaks the previously-green packages/cli suite: sendUpdate stamps segmentId onto every update including replayed history, and history-replayer.test.ts's exact-match expectations were not updated.
失败场景: 任何对该 PR 运行 packages/cli 测试/CI 的场合:HistoryReplayer 通过 Session.sendUpdate 回放 assistant/system 记录,projector 注入 "segmentId": "assistant-uuid:0",10 个 toHaveBeenCalledWith 精确匹配断言因多出字段而失败,workspace 测试命令在所有环境退出码非零。
建议修复: 更新 history-replayer.test.ts 的期望以包含投影的 identity metadata(如同 21k 行 Session.test.ts 的刷新方式),或者如果 segment identity 只应作用于 live session,则在 history-replay 路径跳过投影。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| value === '[link omitted]' || | ||
| value === '[content omitted: export text budget exceeded]' |
There was a problem hiding this comment.
[Critical] Class finding — the export producer can emit values the module's own final validator rejects, so legitimate transcripts hard-fail (invalid_block / envelope_budget_exceeded) instead of degrading like every other unsafe input in this module. Five executed symptoms: (1) safePath yields a bare drive letter 'C:' for Windows drive-root cwds that isSafeExportPath explicitly rejects; (2) once the visible-text budget is exhausted past the 46-byte sentinel window, applyTextBudget returns '' and web_fetch url='' fails the validator; (3) file_read.range passes through unsanitized — a recorded non-integer/negative range throws invalid_block; (4) the text budget accounts raw utf8 bytes while the envelope gate measures JSON-serialized bytes, so escaping inflation lets a document inside every component budget exceed the 32 MB envelope (measured 38.7 MB → throw); (5) a linked inline data-image [](https://…) is spliced into a new remote image by sanitizeMarkdownNavigableUrls' label fallback, and the self-assert throws invalid_markdown_image. — Failure scenario: transcripts with a Windows drive-root tool cwd, long sessions (>8 MB visible text), JSON-escape-heavy tool output, or badge-style linked images fail export outright instead of degrading with diagnostics.
Witness (executed):
safePath('C:\\') -> 'C:' -> isSafeExportPath=false; boundary sweep remaining=47…0 -> THREW invalid_block (url='' arm); range [1.5,10] -> invalid_block; envelope probe serialized=38756037 > 33554432 -> envelope_budget_exceeded; linked data-image -> threw invalid_markdown_image (two independent runs)
Suggested fix: Make producer output always validator-safe: safePath returns '[path]' for bare drive letters; degrade url/labels to the omission markers when the budget would produce ''; sanitize or omit range; account the envelope in the same units as the component budgets (leave headroom for serialization inflation); stop the label-fallback splice from creating new images.
中文说明
类级发现:导出模块的生产者可以产出被模块自身最终校验器拒绝的值,导致合法 transcript 的导出直接抛错(invalid_block / envelope_budget_exceeded)而不是按模块其余部分的设计降级为诊断信息。五个已执行验证的症状:(1) safePath 对 Windows 盘符路径产出裸 'C:',isSafeExportPath 显式拒绝(command preview cwd 路径已实测触发);(2) 可见文本预算耗尽越过 46 字节哨兵窗口后 applyTextBudget 返回 '',web_fetch 的 url='' 无法通过校验器;(3) file_read.range 原样透传,录制的非整数/负数 range 触发 invalid_block;(4) 文本预算按未转义 utf8Bytes 计量而 envelope 门禁按 JSON.stringify 序列化字节计量,转义膨胀使各项分预算全合规的文档仍可能突破 32MB envelope 上限(实测 38.7MB 抛 envelope_budget_exceeded);(5) 链接包裹的内联 data 图片经 sanitizeMarkdownNavigableUrls 的 label 回退拼接生成新的远程图片,最终自校验抛 invalid_markdown_image。
失败场景: Windows 盘符工作目录的会话、长会话(>8MB 可见文本)、含 JSON 转义密集工具输出或含 徽章式 markdown 的 transcript,导出时硬失败而非降级 — 与模块对其余不安全输入的降级语义(占位符+诊断+truncated)矛盾。
建议修复: 让生产者输出永远满足自身校验器:safePath 对裸盘符返回 '[path]';optionalLabel/url 在预算会产生 '' 时降级为省略占位(如 '[link omitted]');range 经 safeCount 规整或省略;预算计量与 envelope 采用同一单位(预留序列化膨胀余量);link-label 拼接不再制造新图片。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const indentedCode = | ||
| fence === undefined && marker === undefined && /^(?: {4}|\t)/.test(line); |
There was a problem hiding this comment.
[Critical] Class finding — the export gate hand-rolls a markdown mini-parser (splitMarkdownFenceSegments + link/image/autolink regexes) to re-derive what a renderer will render over untrusted cross-host documents; CommonMark + inline HTML is an unbounded entrance space. Executed bypasses: (1) a 4-space-indented line is classified indentedCode and skipped, but CommonMark forbids an indented code block from interrupting a paragraph — the lazy-continuation javascript: link / animated GIF is accepted while the identical flat shape is rejected; (2) whitespace around a link/image destination (legal in CommonMark) bypasses the URL gate and the image approval/animation/raster checks; (3) an escaped bang \! falls through both the link model ((?<!!)) and the image model (isEscapedMarkdownSyntax) yet renders as a literal ! followed by an active link; (4) <javascript:…> autolinks, raw HTML (a/iframe/svg) and backtick-in-fence-info shapes likewise pass. The bundled web-shell renderer neutralizes javascript: hrefs itself (defense in depth holds), but the proven defect is this gate, which the PR certifies in place of renderer judgment. — Failure scenario: a cross-host document carrying hello\n [click](javascript:alert(1)), [x]( javascript:alert(1) ), \), or a whitespace-padded animated-GIF data URI passes exportDocumentToTranscriptBlocks/assertExportTranscriptDocumentV1 with empty diagnostics and complete: true — the gate admits exactly the payloads its own invalid_markdown_url/invalid_markdown_image rules exist to reject.
Witness (executed):
F5-GATE-INDENTED: ACCEPTED vs F5-GATE-FLAT: REJECTED invalid_markdown_url (same semantics); N8 padded javascript: link and padded animated GIF: preserved=true, diagnostics=[], complete=true; N9 escaped-bang payload: preserved=true, diagnostics=[], complete=true; marked 15.0.12 renders the lazy continuation as <a href="javascript:alert(1)">
Suggested fix: Stop re-deriving rendering semantics with regexes: parse each markdown text field with the authoritative grammar consumers already depend on (the micromark/remark AST in the dependency tree) and validate structurally — every link/image/autolink destination must pass normalizeNavigableUrl/parseApprovedImageDataUrl, html nodes fail closed, fence metadata respects the rich-render budget. That converts the unbounded enumeration into a closed check over finite node types.
中文说明
类级发现:导出契约门禁用手写 markdown 迷你解析器(splitMarkdownFenceSegments + 链接/图片/autolink 正则)重新推导"渲染器会渲染什么",而 CommonMark+内联 HTML 是无界入口面,已执行验证出多个绕过:(1) 4 空格缩进行被当作 indented code 跳过检查,但 CommonMark 规定缩进代码块不能打断段落——惰性续行中的 javascript: 链接/动画 GIF 被门禁接受(对照的非缩进形态被正确拒绝);(2) 链接/图片目标与括号间的空白(CommonMark 允许)同时绕过 URL 门禁与图片审批/动画拒绝/字节计量;(3) 转义感叹号 ! 同时躲过链接模型((?<!!) 环视)与图片模型(isEscapedMarkdownSyntax),CommonMark 渲染为字面 ! 加活跃链接;(4) javascript:… autolink、原始 HTML(a/iframe/svg)与围栏 info 含反引号等形态同样绕过。捆绑的 web-shell 渲染器自身会中和 javascript: href(纵深防御成立),但被验证的缺陷是这个 PR 用以替代渲染器判断的契约门禁本身。
失败场景: 跨宿主文档携带 hello\n [click](javascript:alert(1))、[x]( javascript:alert(1) )、\) 或空白包裹的动画 GIF data URI 时,通过 exportDocumentToTranscriptBlocks/assertExportTranscriptDocumentV1 且诊断为空、complete:true——门禁放行了它自己的 invalid_markdown_url/invalid_markdown_image 规则意图拦截的载荷。
建议修复: 停止用正则重推导渲染语义:用消费方已依赖的权威语法(依赖树中的 micromark/remark AST)解析每个 markdown 文本字段并做结构化校验——遍历 AST,要求所有 link/image/autolink 目标通过 normalizeNavigableUrl/parseApprovedImageDataUrl,html 节点 fail-closed,围栏元数据遵循富渲染预算。无界枚举即被转换为对有限节点类型的封闭检查。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| }); | ||
| }) | ||
| .join(''); | ||
| const richTaskSafeValue = resourceSafeValue.replace( |
There was a problem hiding this comment.
[Critical] The rich-render budget pass re-parses the rejoined text with its own fence regex instead of reusing splitMarkdownFenceSegments, and its counting disagrees with both the validator and the renderer: (1) fence-looking lines that are literal content inside an already-open code block are counted as render tasks and, over budget, rewritten — injecting [source fallback: js] text that was never in the conversation while truncated stays false; (2) the producer counts ```text/```plain/```plaintext fences against maxRichRenderTasks, while the validator's recount and the renderer (which gives those languages no rich processing) both exclude them — real rich fences (js/mermaid) are demoted up to 100 tasks early. — Failure scenario: a document with >100 info-string fences (e.g. a transcript quoting js examples inside a md block): user-visible [source fallback: …] garbage inside literal code samples, a spurious rich_render_budget_exceeded warning, and premature demotion of genuinely rich fences.
Witness (executed):
probe: 100 ```text fences + 1 ```js fence -> the js fence demoted to "```text [source fallback: js]" (observed verbatim in the exported block text) with rich_render_budget_exceeded emitted, while the document still passed its own self-assert (validator counts only truly-rich fences)
Suggested fix: Make splitMarkdownFenceSegments the single authoritative fence parse: record which lines are genuine fence openers (marker seen while no fence is open) and run the rich-render count/demotion over that record — rewriting only real opening lines' info strings — excluding text/plain/plaintext consistently with the validator and renderer.
中文说明
富渲染预算通道用自己的围栏正则重新解析拼接后的文本,而不是复用 splitMarkdownFenceSegments,且计数口径与校验器/渲染器不一致:(1) 已开启代码块内部的"围栏样"字面内容行被当作渲染任务计数、超预算时被改写——向导出文本注入从未存在过的 "text [source fallback: js]" 内容而 truncated 保持 false;(2) 生产者把 text/plain/plaintext 围栏计入 maxRichRenderTasks,而校验器的重算与渲染器(resolveFenceLanguage 对其不做富处理)都排除这三种语言——两把预算尺不一致,真实的富围栏(js/mermaid)被提前降级。
失败场景: 含 100+ 信息串围栏(含文本中引用 ```js 代码示例的 markdown 教程类转录)的文档:字面代码示例中被注入 [source fallback: …] 垃圾文本、发出虚假的 rich_render_budget_exceeded 警告,且 js/mermaid 围栏最多多提前 100 个任务被降级。
建议修复: 让 splitMarkdownFenceSegments 成为唯一权威围栏解析:记录真正的围栏开启行(在 fence === undefined 时见到 marker),富渲染计数/降级只在该记录上进行;并在计数中排除 text/plain/plaintext(与校验器和渲染器一致)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const VISIBLE_SYSTEM_RECORD_SUBTYPES = new Set([ | ||
| 'notification', | ||
| 'cron', |
There was a problem hiding this comment.
[Critical] VISIBLE_SYSTEM_RECORD_SUBTYPES omits 'slash_command', but the replay machine (projectSystemRecord, transcript-replay.ts:967-1008) does project slash_command records as visible assistant messages (/about, /help output and goal cards recorded via restoreGoal.ts:235). The policy rejects those records before projection with only an info-severity diagnostic and complete unaffected — contradicting the design doc §8.2 "若原本用户可见则门禁失败". — Failure scenario: every export containing slash-command output silently drops it: probe shows the tail slash record absent from the export with diagnostics record_internal_excluded (info) and complete: true, truncated: false; with a later record parented on it, a misattributed causal_record_excluded error instead — while the canonical projection with the record accepted renders the content as an assistant block.
Witness (executed):
N3(tail): contains ABOUT_VISIBLE_OUTPUT=false | diagnostics=[{code:record_internal_excluded,severity:info}] | complete=true; N3b canonical projection: contains=true; N3(later record): causal_record_excluded error
Suggested fix: Add 'slash_command' to VISIBLE_SYSTEM_RECORD_SUBTYPES, or emit an affectsCompleteness diagnostic whenever a projection-capable subtype is excluded.
中文说明
VISIBLE_SYSTEM_RECORD_SUBTYPES 遗漏 'slash_command',但回放机(transcript-replay.ts projectSystemRecord)确实把 slash_command 记录投影为可见 assistant 消息(/about、/help 输出与 restoreGoal.ts 记录的 goal 卡片)。策略在投影前以 info 级诊断拒绝这些记录且 complete 不受影响——与设计文档 §8.2 "若原本用户可见则门禁失败" 直接矛盾。
失败场景: 任何含斜杠命令输出的 transcript 导出时静默丢失这些内容:探针实测尾部 slash 记录 -> 输出缺失、diagnostics 仅 record_internal_excluded(info)、complete:true truncated:false;若其后还有记录则误报 causal_record_excluded(error)。而同一记录走规范投影时确实渲染为 assistant 块(contains ABOUT_VISIBLE_OUTPUT: true)。
建议修复: 将 'slash_command' 加入 VISIBLE_SYSTEM_RECORD_SUBTYPES;或对任何可投影但被排除的 subtype 发出影响完整性(affectsCompleteness)的诊断。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| "complete": { "type": "boolean" }, | ||
| "truncated": { "type": "boolean" }, | ||
| "projectName": { "type": "string", "maxLength": 400 }, | ||
| "repository": { "type": "string", "maxLength": 200 }, |
There was a problem hiding this comment.
[Suggestion] The pinned schema caps metadata.repository at 200 chars, but the runtime import gate (assertMetadata → isSafeRepository → isSafeDisplayUrl) applies no length cap to http(s) URLs — the machine-readable contract is stricter than the gate it mirrors, so schema-conformant documents are rejected on import. (The producer side is already safe: over-long gitRepo degrades to '[link omitted]'; the drift direction is gate-accepts/schema-forbids.) — Failure scenario: a cross-host document whose repository is a 248-char https URL: schema validation rejects (maxLength 200) while the runtime gate accepts — consumers implementing against the schema disagree with runtime behavior.
Witness (executed):
probe: 248-char repository -> unmodified PR accepts (runtimeError=none) while ajv against the pinned schema rejects; adding isSafeLabel(value,200) to assertMetadata flips to invalid_metadata (reverted)
Suggested fix: Add a length cap to isSafeRepository's URL branch (e.g. isSafeLabel(value, 200)) to align with the schema.
中文说明
固定 schema 把 metadata.repository 上限设为 200 字符,但运行时导入门禁(assertMetadata -> isSafeRepository -> isSafeDisplayUrl)对 http(s) URL 无长度上限——机器可读契约比它镜像的运行时门禁更严格:schema 合规文档在导入时被拒绝。(生产者侧已有防护:createExportTranscriptDocumentV1 对超长 gitRepo 输出 '[link omitted]',漂移方向是门禁接受/schema 拒绝。)
失败场景: repository 为 248 字符 https URL 的跨宿主文档:schema 校验拒绝(maxLength 200),运行时门禁接受——契约两面不一致,消费方按 schema 实现会与运行时行为冲突。
建议修复: 在 isSafeRepository 的 URL 分支加长度上限(如 isSafeLabel(value, 200)),与 schema 对齐。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| block.kind === 'tool' && | ||
| (block.status === 'completed' || block.status === 'failed') && | ||
| block.resultPreview === undefined |
There was a problem hiding this comment.
[Suggestion] The "completed/failed tool without resultPreview" branch of hasExplicitContentLoss in assertDocumentConsistency is unreachable: assertExportBlock (line 310 loop) hard-rejects that exact shape before assertDocumentConsistency (line 328) runs — the two validation layers encode contradictory semantics for one shape (layer 1: invalid, throw; layer 2: degradable explicit content loss), and only the "invalid" layer can fire. (The serializer independently never emits the shape — it substitutes '[tool result omitted from export]' — which is why the contradiction stayed latent.) — Failure scenario: if a future change relaxes assertExportBlock for that shape (e.g. to allow degradation), the contradiction becomes live immediately; today the dead branch masks the design disagreement.
Suggested fix: Delete the unreachable branch, or relax assertExportBlock for the shape and let assertDocumentConsistency enforce complete: false/truncated: true (if degradable content loss is the intent).
中文说明
assertDocumentConsistency 的 hasExplicitContentLoss 中 "completed/failed 工具无 resultPreview" 分支不可达:同一 assertExportTranscriptDocumentV1 管线中 assertExportBlock(第 310 行循环)先于 assertDocumentConsistency(第 328 行)对该形状硬抛 invalid_block——两个验证层对同一形状编码矛盾语义(一层:非法,抛错;另一层:可降级的显式内容丢失),只有"非法"层可能触发。(序列化器独立地从不产出该形状——替换为 '[tool result omitted from export]'——因此矛盾一直潜伏。)
失败场景: 维护者若使 assertExportBlock 放宽该形状(例如允许降级),assertDocumentConsistency 的语义与其冲突将立即显形;当前该死分支掩盖了两层设计矛盾。
建议修复: 删除不可达分支,或让 assertExportBlock 放宽该形状并由 assertDocumentConsistency 强制 complete:false/truncated:true(若其本意是可降级内容丢失)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| function appendSubTool( | ||
| parent: DaemonMessageToolCall, | ||
| toolCall: DaemonMessageToolCall, | ||
| includeSourceIdentity: boolean, | ||
| ): void { |
There was a problem hiding this comment.
[Suggestion] A parent (sub-agent) tool's sourceBlockIds closure is only populated at appendSubTool time; block ids later added to a subtool by mergeToolCall(..., {collectSourceIdentity}) never propagate back to the parent tool, and synchronizeToolGroupSourceIdentity repairs only the message level — so the parallel-agents probe evidence (transcriptRenderProbe.ts:72-77 reads the agent tool's sourceBlockIds directly) under-attributes rendered content. — Failure scenario: a permission block merged into a subtool by callId has its rendered effect (subtool status/permission history inside the agent card) present in the UI but absent from the probe evidence's sourceBlockIds — the exact-coverage contract fails (or must be loosened) for such transcripts.
Witness (executed):
probe: messageLevel includes permission-block; parentToolLevel misses it; probeEvidence (parallel_agents item) misses it — a parent-propagation fix arm makes all three levels agree (144/144 green, fix reverted)
Suggested fix: Track the parent at appendSubTool and re-union subtool ids into the parent's sourceBlockIds after each mergeToolCall (or have the probe read the message-level union).
中文说明
父(sub-agent)工具的 sourceBlockIds 闭包只在 appendSubTool 时填充;之后经 mergeToolCall(..., {collectSourceIdentity}) 加到子工具上的块 id 从不回传父工具,synchronizeToolGroupSourceIdentity 只修复消息层——并行代理探针证据(transcriptRenderProbe.ts:72-77 直接读代理工具的 sourceBlockIds)对已渲染内容归属不足。
失败场景: 权限块经 callId 并入子工具后,其渲染效果(代理卡片内的子工具状态/权限历史)出现在界面却不在探针证据的 sourceBlockIds 中——精确覆盖契约对这类转录失败或被迫放宽。
建议修复: 在 appendSubTool 记录父引用,并在每次 mergeToolCall 后把子工具新增 id 回并入父工具 sourceBlockIds(或让探针改读消息层并集)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| expect(renderedSourceBlockIds).toEqual( | ||
| new Set(exportDocument.blocks.map((block) => block.id)), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The exact-coverage contract (every source block id must appear in some rendered item's sourceBlockIds) has no exclusion mechanism, but several block shapes are deliberately never rendered in document/safe mode: empty assistant blocks without usage (if (!textBlock.text && !textBlock.usage) break;), unresolved permission blocks (pending-permission UI is live-only; WebShellTranscript passes pendingApproval={null}), and AskUserQuestion permission blocks (permissionBlockToToolCall returns undefined; rememberPermissionToolInfo never ledgers the permission block's own id). The export schema explicitly supports permission blocks, so real exports can carry these shapes. (One correction: thought blocks are NOT dropped — an empty thought still yields a thinking message carrying sourceBlockIds.) — Failure scenario: any fixture or real transcript containing one of these shapes fails the gate with no way to express "deliberately unrendered"; today only fixtures avoiding these shapes pass.
Witness (executed):
probe: 5 blocks including all three shapes -> rendered ids miss [empty-assistant, pending-permission, ask-user-permission]
Suggested fix: Ledger deliberately-unrendered blocks onto an explicit evidence field (e.g. unrenderedBlockIds) the gate subtracts, or scope the assertion to renderable blocks — then add fixture cases for each skipped shape.
中文说明
精确覆盖契约(每个源块 id 必须出现在某个渲染项的 sourceBlockIds)没有排除机制,但若干块形状在 document/safe 模式下刻意不渲染:无 usage 的空 assistant 块(if (!textBlock.text && !textBlock.usage) break;)、未解决权限块(pending 权限 UI 仅 live;WebShellTranscript 传 pendingApproval={null})、AskUserQuestion 权限块(permissionBlockToToolCall 返回 undefined,rememberPermissionToolInfo 从不记录权限块自身 id)。导出 schema 明确支持 permission 块,真实导出可携带这些形状。(一处更正:thought 块不被丢弃——空 thought 仍产生带 sourceBlockIds 的 thinking 消息。)
失败场景: 任何含上述形状的夹具/真实转录使门禁失败且无法表达"刻意不渲染";当前只有避开这些形状的夹具能通过。
建议修复: 把刻意不渲染的块记入显式证据字段(如 unrenderedBlockIds)供门禁扣除,或把断言限定到可渲染块;并为每种被跳过形状增加夹具用例。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| :global([data-transcript-render-mode='document']) .message { | ||
| max-height: none; | ||
| overflow: visible; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The document-mode override un-clips the shell-message container (.message) but leaves its .command child with overflow: hidden; text-overflow: ellipsis; white-space: nowrap — long shell commands are truncated to one ellipsized line in exported documents while the output below is fully expanded and every sibling surface (DiffView .lines, SubAgentPanel .stream/.scrollWindow, ToolChrome .expandedOutput, the MessageList viewport) is deliberately un-clipped by this same change; the command's only render site is this span (no title or alternate full-text path). — Failure scenario: a transcript document containing a long command (docker/npm/git with many flags — routinely 100+ chars at document widths) renders npm install --save pkg-a pkg-b … with an ellipsis; the full command is unrecoverable from the document.
| :global([data-transcript-render-mode='document']) .message { | |
| max-height: none; | |
| overflow: visible; | |
| } | |
| :global([data-transcript-render-mode='document']) .message { | |
| max-height: none; | |
| overflow: visible; | |
| } | |
| :global([data-transcript-render-mode='document']) .command { | |
| overflow: visible; | |
| text-overflow: unset; | |
| white-space: pre-wrap; | |
| } |
中文说明
document 模式覆盖解开了 shell 消息容器(.message),但其 .command 子元素仍保持 white-space:nowrap; overflow:hidden; text-overflow:ellipsis 且文件内无任何覆盖——长 shell 命令在导出文档中被截断为单行带省略号,而同一改动把下方 .output(pre-wrap)及所有兄弟表面(DiffView .lines、SubAgentPanel .stream/.scrollWindow、ToolChrome .expandedOutput、MessageList viewport)完全展开;命令的唯一渲染点就是该 span(无 title 或全文替代路径)。
失败场景: 含长命令(docker/npm/git 多参数,文档宽度下常见 100+ 字符)的转录文档:头部渲染 'npm install --save pkg-a pkg-b …' 带省略号,完整命令在文档中不可恢复。
建议修复: 扩展同一 document 覆盖块::global([data-transcript-render-mode='document']) .command { overflow: visible; text-overflow: unset; white-space: pre-wrap; }
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
@qwen-code /resolve |
1 similar comment
|
@qwen-code /resolve |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
What this PR does
This PR establishes and validates a minimum read-only transcript contract for Web Shell, Qwen Tauri Desktop, VS Code, and future HTML export while continuing to use the existing daemon transcript blocks instead of introducing a second public message model.
It adds versioned synthetic fixtures, schemas, hashes, a capability matrix, stable identity and render/action probes, and machine-generated gates for shared transcript semantics. Both the direct-daemon and ACP candidates pass append, partial-prepend, replay, scope, render, and action identity validation; ACP remains the selected path for a later VS Code migration because it is the current production transport.
It also introduces a versioned, allowlisted export document boundary and an opt-in document rendering mode with typed tool/result projections, resource budgets, CSP and zero-network checks, path and metadata sanitization, and document-only Mermaid limits. The default interactive and readonly adapters retain their existing raw input/output behavior, including complete
write_fileTurn Output diffs.This is contract prevalidation and supporting infrastructure. It does not migrate the production VS Code timeline, replace the current HTML export renderer, change composer or permission workflows, or introduce a shared ChatPanel package.
Why it's needed
The four target hosts currently obtain and render conversation history through different data paths. Before sharing more presentation code, they need repeatable evidence that the existing transcript representation preserves the same visible semantics, stable identity, and host-action targets across replay, reconnect, pagination, and protocol boundaries.
HTML export also requires a stricter trust boundary than an interactive runtime: raw tool payloads, credentials, internal metadata, absolute paths, and active remote resources must not leak into an exported document. Establishing these identity, compatibility, and export-safety gates first provides a measurable foundation for later VS Code and HTML migrations without causing a Web Shell behavior break.
Reviewer Test Plan
How to verify
write_filecall still produces the complete file diff and line statistics.Evidence (Before & After)
Tested on
Environment (optional)
macOS 26.0, Node.js v22.22.3, npm 10.9.8, and the repository-pinned Playwright Chromium. Validation completed with the repository build and typecheck, 644 CLI Session/identity tests, and 7 cross-host contract/document integration tests. The generated report recorded
overall: pass, direct-daemon and ACP aspass,selectedVscodePath: acp, zero blockers, zero unexpected requests, and zero CSP violations.Risk & Scope
Linked Issues
Closes #9354
中文说明
Establish cross-host chat transcript contract prevalidation
What this PR does
本 MR 为 Web Shell、Qwen Tauri Desktop、VS Code 和未来的 HTML 导出建立并验证最小只读 transcript 契约,同时继续复用现有 daemon transcript blocks,不引入第二套公共消息模型。
它增加了版本化合成 fixtures、schemas、hashes、capability matrix、稳定 identity 与 render/action probes,以及共享 transcript 语义的机器生成门禁。direct-daemon 与 ACP 两条候选路径均通过 append、partial prepend、replay、scope、render 和 action identity 验证;由于 ACP 是当前生产传输路径,后续 VS Code 迁移仍选择 ACP。
它还引入了版本化、逐字段 allowlist 的导出文档边界,以及显式启用的 document rendering mode,包括类型化工具与结果投影、资源预算、CSP 与零网络检查、路径和 metadata 脱敏,以及仅作用于 document mode 的 Mermaid 限制。默认 interactive 和 readonly adapters 保持原有 raw input/output 行为,包括完整的
write_fileTurn Output diff。本 MR 的范围是契约预验证和配套基础设施。它不迁移生产 VS Code 时间线、不替换当前 HTML 导出 renderer、不改变 composer 或权限工作流,也不引入共享 ChatPanel package。
Why it's needed
四个目标宿主目前通过不同数据路径获取和渲染会话历史。在进一步共享展示代码之前,需要用可重复的证据证明现有 transcript 表达能够在 replay、reconnect、pagination 和协议边界下保持一致的可见语义、稳定 identity 与宿主动作目标。
HTML 导出还需要比交互式运行时更严格的信任边界:raw tool payload、credential、内部 metadata、绝对路径和主动远程资源都不能泄漏到导出文档。先建立这些 identity、兼容性和导出安全门禁,可以为后续 VS Code 与 HTML 迁移提供可度量的基础,同时避免造成 Web Shell 行为回归。
Reviewer Test Plan
How to verify
write_file调用仍能生成完整文件 diff 与行数统计。Evidence (Before & After)
N/A — 这是契约、安全和重构基础设施,没有刻意引入用户可见 UI 变化,因此未生成截图或录屏。
Tested on
Environment (optional)
macOS 26.0、Node.js v22.22.3、npm 10.9.8,以及仓库锁定的 Playwright Chromium。已完成仓库 build 与 typecheck、644 个 CLI Session/identity tests,以及 7 个跨宿主 contract/document integration tests。生成的报告记录为
overall: pass,direct-daemon 和 ACP 均为pass,selectedVscodePath: acp,blockers 为空,非预期请求为零,CSP 违规为零。Risk & Scope
Linked Issues
Closes #9354