Skip to content

feat(web-shell): add transcript contract prevalidation - #9388

Merged
yiliang114 merged 6 commits into
QwenLM:mainfrom
water-in-stone:codex/chat-transcript-contract-prevalidation
Aug 19, 2026
Merged

feat(web-shell): add transcript contract prevalidation#9388
yiliang114 merged 6 commits into
QwenLM:mainfrom
water-in-stone:codex/chat-transcript-contract-prevalidation

Conversation

@water-in-stone

Copy link
Copy Markdown
Collaborator

What this PR does

This PR establishes the evidence-only first stage of a shared chat transcript contract for Web/Qwen Server, Qwen Tauri Desktop, VS Code, and HTML Export. It freezes deterministic synthetic fixtures, hashes, a closed V1 export schema, and a capability matrix; verifies the existing ChatRecord projection and default Web Shell interactive/read-only raw semantics; and adds a regression for preserving the complete write_file Turn Output diff when a safe preview is also present.

It also probes the current direct-daemon and ACP candidates under partial history prepend. Both candidates reproducibly fail the stable identity gate, so the overall result remains fail and selectedVscodePath remains null. No VS Code timeline migration, export builder, document render mode, protocol field, or production stable-identity projector is included. The accompanying overall design defines those MR2 responsibilities without introducing a second runtime message model in MR1.

Why it's needed

The future VS Code and HTML Export migrations need a repeatable contract before they can safely reuse the Web Shell transcript renderer. The current direct-daemon and ACP paths derive ordinal block identities that change when earlier history is prepended, while export also requires an explicitly versioned and allowlisted boundary that cannot expose runtime raw payloads. Recording these facts as deterministic evidence prevents a green test run from being mistaken for migration readiness and keeps production changes out of the prevalidation stage.

Reviewer Test Plan

How to verify

  1. Run the build and typecheck commands below and expect both to complete successfully.
  2. Run the transcript contract test and expect fixture hashes, the closed export schema, current ChatRecord/Web Shell semantics, and Desktop packaging wiring to validate. Expect the gate evidence itself to remain overall: "fail", with both direct-daemon and ACP marked fail and no VS Code path selected.
  3. Run the focused Web Shell test and expect a write_file block containing both full raw content and a shortened preview to produce the complete Turn Output diff from the raw content.
npm run build
npm run typecheck
cd integration-tests && npx vitest run chat-transcript-contract.test.ts
cd ../packages/web-shell && npx vitest run client/components/artifacts/turnOutputSelectors.test.ts

Evidence (Before & After)

Tested on

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

Environment (optional)

Risk & Scope

  • Main risk or tradeoff: The tests are intentionally green while asserting a failed migration gate; reviewers must not interpret test success as stable identity, a selected VS Code path, or a completed HTML Export pipeline.
  • Not validated / out of scope: Production direct-daemon/ACP identity fixes, the full append/prepend/replay matrix, VS Code and VSIX behavior, the export builder, document-mode browser behavior, CSP and zero-network enforcement, resource budgets, and Desktop installation smoke testing are deferred to MR2.
  • Breaking changes / migration notes: No production transcript pipeline or default interactive/read-only raw semantics are intentionally changed in this PR. VS Code migration and HTML Export require a separate consumer-driven MR after the documented gates pass.

Linked Issues

#9387

中文说明

What this PR does

本 PR 建立 Web/Qwen Server、Qwen Tauri Desktop、VS Code 与 HTML Export 共享聊天 transcript 契约的第一阶段纯证据基础。它冻结确定性的合成 fixtures、hash、封闭的 V1 导出 schema 和 capability matrix;验证现有 ChatRecord 投影以及 Web Shell 默认 interactive/read-only 模式的 raw 语义;并增加回归覆盖,确保同时存在安全 preview 时,write_file 的 Turn Output 仍保留完整文件 diff。

它还在 partial history prepend 场景下探测当前 direct-daemon 和 ACP 两条候选路径。两者都能稳定复现 stable identity 门禁失败,因此整体结果继续保持 failselectedVscodePath 继续保持 null。本 PR 不包含 VS Code 时间线迁移、export builder、document render mode、协议字段或生产 stable-identity projector。配套的唯一总体设计定义了这些 MR2 职责,但 MR1 不会引入第二套运行时消息模型。

Why it's needed

未来的 VS Code 与 HTML Export 迁移需要先建立可重复的契约证据,才能安全复用 Web Shell transcript renderer。当前 direct-daemon 和 ACP 路径生成的 ordinal block identity 会在 prepend 更早历史时发生变化,而导出路径还需要一个明确版本化且基于 allowlist 的边界,以避免暴露 runtime raw payload。将这些事实记录为确定性证据,可以防止把测试绿色误认为迁移已经就绪,并避免在预验证阶段引入生产改动。

Reviewer Test Plan

How to verify

  1. 运行下面的 build 和 typecheck 命令,预期两者均成功完成。
  2. 运行 transcript contract 测试,预期 fixture hash、封闭的 export schema、当前 ChatRecord/Web Shell 语义以及 Desktop 打包 wiring 均通过验证。门禁证据本身应继续保持 overall: "fail",direct-daemon 和 ACP 都应为 fail,并且不选择任何 VS Code 路径。
  3. 运行 Web Shell 定向测试,预期同时包含完整 raw content 和缩短 preview 的 write_file block 会使用 raw content 生成完整的 Turn Output diff。
npm run build
npm run typecheck
cd integration-tests && npx vitest run chat-transcript-contract.test.ts
cd ../packages/web-shell && npx vitest run client/components/artifacts/turnOutputSelectors.test.ts

Evidence (Before & After)

N/A — 本 PR 增加契约证据和回归覆盖,不包含用户可见的 UI 变化。源文档未提供截图、录屏或 before/after 日志。

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️ not tested (OS not provided)
🐧 Linux ⚠️ not tested (OS not provided)

Environment (optional)

N/A — 源文档记录了这些验证命令已经执行,但没有提供 runtime、sandbox 或操作系统信息。

Risk & Scope

  • Main risk or tradeoff: 测试会正常通过,但它们有意断言迁移门禁失败;评审者不能把测试成功理解为 stable identity 已完成、VS Code 路径已选定或 HTML Export 管线已实现。
  • Not validated / out of scope: 生产 direct-daemon/ACP identity 修复、完整 append/prepend/replay matrix、VS Code 与 VSIX 行为、export builder、document-mode 浏览器行为、CSP 与零网络约束、资源预算以及 Desktop 安装产物 smoke testing 均推迟到 MR2。
  • Breaking changes / migration notes: 本 PR 不会有意修改生产 transcript pipeline 或默认 interactive/read-only raw 语义。VS Code 迁移与 HTML Export 必须在文档门禁通过后,通过独立且由真实消费者驱动的 MR 完成。

Linked Issues

N/A


heyang.why and others added 2 commits August 18, 2026 14:19
Freeze reproducible evidence for current transcript paths before any
VS Code or HTML export production migration.

- Add versioned fixtures, closed export schema, and capability gates
- Probe direct-daemon and ACP identity under partial history prepend
- Preserve raw adapter semantics and full write_file Turn Output diffs
- Document the two-MR architecture, security constraints, and blockers
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 18, 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 — and for the rework: this is the evidence-only split of #9355 that review there asked about, and the ~3.1k lines of production-shaped surface (export builder, probes, wire-format fields) are indeed gone. A few honest gate observations before anyone goes deeper.

  • Template: complete ✓ (bilingual, test plan included).
  • Problem: no observed defect. The linked issue (Prevalidate a shared chat transcript contract across hosts #9387) was filed by the PR author shortly before this PR, is still status/needs-triage, and has no maintainer sign-off. The motivation is forward-looking — freeze evidence before a future VS Code timeline / HTML export migration — not a user-visible break. For a feat that doesn't disqualify, but it puts all the weight on the direction question below.
  • Direction: this is the same direction question feat(transcript): add cross-host contract prevalidation #9355 deferred to maintainers yesterday, and it is still unanswered: do we want prevalidation infrastructure — a closed V1 export schema, a capability matrix, gate fixtures — for migrations (VS Code timeline, HTML export rebuild) that have not been scoped or approved anywhere yet? The manifest declares web, tauri, vscode, html as the contract's consumers, which is a product-direction and public-contract call that belongs to a maintainer, not to the gate. Claude Code's CHANGELOG has no comparable prevalidation practice — no external signal either way.
  • Size: no core paths touched — the only non-test, non-doc code change is a 4-line vitest alias for an SDK subpath (./daemon/transcript) that already exists in the package exports. For the record: ~4 production-config lines, ~381 test lines, ~828 schema lines, ~955 design-doc/fixture lines. No size gate applies.
  • Approach: the split itself is the right response to feat(transcript): add cross-host contract prevalidation #9355's review, and the new write_file full-diff regression test in web-shell has standalone value — it pins current behavior regardless of any migration. But the bulk of the diff still only pays off if MR2 happens: the "gate" result is largely hardcoded in the test (the blocker strings, the overall: fail, the matrix assertions); the one thing actually measured — ordinal block IDs shift when history is prepended — is inherent to ordinal IDs and doesn't need frozen fixtures to establish. If the direction gets a green light, consider landing the standalone regression test plus a compact current-semantics suite first, and keeping the export schema / capability matrix / MR plan in the issue until a migration PR actually needs them.
  • Risk: no elevated risk signals — no high-risk paths matched; the diff is tests, fixtures, schemas, and a design doc.
  • CI: fork workflows were authorized and still running on the reviewed commit at gate time (ubuntu unit suite and web-shell visuals in progress; macOS/Windows test jobs skipped by path filters; desktop-shell checks green).

⏸️ Flagging these for discussion before diving deeper — and escalating the direction call to @wenshao @pomelo-nwu: the prevalidation-before-migration question was deferred on #9355 and never answered; this PR is much smaller and cleaner, but the question is the same. Stopping the pipeline here per the gate rules; a maintainer's direction (or a re-run via @qwen-code /triage once decided) picks it back up.

中文说明

感谢贡献——也感谢这次重构:这正是 #9355 评审中询问的"纯证据"拆分版本,约 3.1k 行生产形态的代码(export builder、probe、wire-format 字段)确实都移除了。在深入之前,给出几点如实的门禁观察。

  • 模板:完整 ✓(双语,含测试计划)。
  • 问题:没有观测到的缺陷。关联 issue(Prevalidate a shared chat transcript contract across hosts #9387)由 PR 作者在本 PR 前不久自行提交,目前仍是 status/needs-triage,没有 maintainer 表态。动机是前瞻性的——在未来的 VS Code 时间线 / HTML 导出迁移之前冻结证据——而不是已发生的用户可见问题。对 feat 类 PR 这并不构成否决,但全部重量都落到了下面的方向问题上。
  • 方向:这与 feat(transcript): add cross-host contract prevalidation #9355 昨天转交 maintainer 的方向问题是同一个,且仍未得到答复:我们是否要为尚未立项或批准的迁移(VS Code 时间线、HTML 导出重构)建设预验证基础设施——封闭的 V1 导出 schema、capability matrix、门禁 fixtures?manifest 声明 webtaurivscodehtml 为该契约的消费方,这是产品方向与公共契约层面的决策,应由 maintainer 做出,而不是门禁。Claude Code 的 CHANGELOG 中没有可比的预验证实践——没有外部方向信号。
  • 规模:未触及核心路径——唯一非测试、非文档的代码改动是 4 行 vitest 别名,指向 SDK 包导出中已存在的子路径(./daemon/transcript)。供参考:约 4 行生产配置、约 381 行测试、约 828 行 schema、约 955 行设计文档/fixture。不触发任何规模门禁。
  • 方案:这次拆分本身是对 feat(transcript): add cross-host contract prevalidation #9355 评审的正确回应,web-shell 中新增的 write_file 完整 diff 回归测试也有独立价值——无论是否有迁移,它都固化了当前行为。但 diff 的主体仍然只有在 MR2 发生时才有回报:"门禁"结果在很大程度上是测试里硬编码的(blocker 字符串、overall: fail、matrix 断言);唯一实际被测量的事实——prepend 历史时 ordinal block ID 会变化——是 ordinal ID 的固有属性,不需要冻结 fixtures 来证明。如果方向获得批准,可以考虑先落地独立的回归测试加一个紧凑的当前语义套件,把 export schema / capability matrix / MR 计划留在 issue 里,等迁移 PR 真正需要时再引入。
  • 风险:无升级风险信号——未命中高风险路径;diff 仅包含测试、fixtures、schema 和设计文档。
  • CI:fork 工作流已获授权,门禁检查时仍在评审 commit 上运行(ubuntu 单测套件与 web-shell 视觉采集进行中;macOS/Windows 测试任务被路径过滤跳过;desktop-shell 检查通过)。

⏸️ 先提出讨论,再深入看代码——并将方向决策转交 @wenshao @pomelo-nwu:预验证先于迁移的问题在 #9355 上已被转交且从未得到答复;本 PR 更小更干净,但问题相同。按门禁规则在此停止流水线;待 maintainer 给出方向后(或届时通过 @qwen-code /triage 重跑)可继续。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 18, 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 f055ab3. 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 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.

Not explored to full depth (tool budget reached): "agent 3b": repo-root npm run typecheck across all workspaces was not run; I verified web-shell's and integration-tests' typecheck programs individually instead..

中文说明

未探索到全部深度(达到工具调用预算):"agent 3b"repo-root npm run typecheck across all workspaces was not run; I verified web-shell's and integration-tests' typecheck programs individually instead.

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

Comment thread integration-tests/chat-transcript-contract.test.ts
Comment thread integration-tests/chat-transcript-contract.test.ts
Comment thread integration-tests/chat-transcript-contract.test.ts Outdated
Comment thread integration-tests/chat-transcript-contract.test.ts Outdated
Comment thread integration-tests/chat-transcript-contract.test.ts
Comment thread integration-tests/chat-transcript-contract.test.ts
Comment thread integration-tests/chat-transcript-contract.test.ts
Comment thread integration-tests/chat-transcript-contract.test.ts Outdated
Make the evidence-only contract suite enforce the review assumptions it
documents while preserving the existing runtime transcript behavior.

- Run the contract suite in the required no-AK integration job
- Fail closed on ambiguous identity probes and deduplicate gate kinds
- Enforce manifest, hash, export safety, and renderer version boundaries
- Cover visible transcript text and stable Desktop packaging semantics
- Record the complete PR comment evaluation and verification outcome
@water-in-stone

Copy link
Copy Markdown
Collaborator Author

Addressed all 11 actionable inline findings in commit 494fa98 and resolved the corresponding threads after replying with per-finding evidence.

Key outcomes:

  • the integration TypeScript program and root typecheck both pass;
  • the contract suite is now part of the required no-AK curated command and its wiring is locked by a script test;
  • identity probes fail closed on ambiguous/unsupported alignment and record kind sets;
  • manifest/schema/hash, frozen error kinds, visible text, export tripwires, Desktop wiring, and renderer version boundaries are now enforced;
  • after rebuilding the bundle, the full no-AK run passed 13/13 files and 146/146 tests, including the contract suite at 5/5;
  • npm run build, npm run bundle, the Turn Output regression suite (22/22), and formatting/lint checks pass.

The complete Chinese evaluation, including the original comments and diff annotations, is committed as pr-9388-comment-evaluation.md.

Regarding the direction question in #issuecomment-5324569450: this remains an evidence-only MR. It does not add a production transcript model, export builder, document mode, VS Code timeline migration, protocol fields, or transport selection. The two partial-prepend identity candidates intentionally remain FAIL and selectedVscodePath remains null. Whether this prevalidation infrastructure should land before the migration is still a maintainer decision; the code changes above do not pretend to decide that product/architecture question.

@qwen-code /review

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline.

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

  • integration-tests/fixtures/chat-transcript-contract/v1/schema/export-transcript-document-v1.schema.json:381 — [probe] Frozen schema's base64 guards admit undecodable payloads (rasterImage.data :381, thumbnailUrl :626) — anchored on code unc…
  • docs/design/web-shell/chat-transcript-contract-prevalidation.md:658 — [review] §12.1's MR1 whitelist excludes web-shell production code, yet this PR modifies three web-shell production files it never discloses
  • docs/design/web-shell/chat-transcript-contract-prevalidation.md:190 — [review] §5.2's must-express semantics table omits the debug block kind the baseline renders user-visible
  • docs/design/web-shell/chat-transcript-contract-prevalidation.md:650 — [review] §11.3 prescribes 8 capability-matrix fields; the shipped matrix has 5 columns
  • integration-tests/fixtures/chat-transcript-contract/v1/schema/export-transcript-document-v1.schema.json:725 — [probe] Required exportedAt is guarded only by annotation-only format: date-time (no minLength/pattern, no format-asserting valida…
  • integration-tests/chat-transcript-contract.test.ts:32 — [probe] manifest.expectedDiagnostics floats free of the probe output — pinned only against a duplicated hardcoded literal
  • docs/design/web-shell/chat-transcript-contract-prevalidation.md:316 — [review] §8's TranscriptRenderedItemEvidence interface is declared mandatory but never instantiated; its layer has no matrix row, fixture field, or probe
  • integration-tests/chat-transcript-contract.test.ts:324 — [review] additionalProperties closure asserted only on the two top-level objects, never on any $defs entry
  • docs/design/web-shell/chat-transcript-contract-prevalidation.md:652 — [probe] §11.3's closed Gate vocabulary is contradicted by the frozen matrix's hedged 'PASS foundation only' Desktop-reuse cell (1 of 9 rows violates)
中文说明

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

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

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

Comment thread integration-tests/chat-transcript-contract.test.ts
Comment thread integration-tests/chat-transcript-contract.test.ts
Comment thread integration-tests/chat-transcript-contract.test.ts Outdated
Comment thread packages/web-shell/client/utils/todos.ts
Comment thread pr-9388-comment-evaluation.md Outdated
Comment thread pr-9388-comment-evaluation.md Outdated
@water-in-stone

Copy link
Copy Markdown
Collaborator Author

Round 2 review suggestions are addressed in 34e7a7f. All seven inline threads have individual replies and are resolved. Local verification passed: transcript contract tests (5/5), integration TypeScript compilation, CI wiring tests (9 passed, 1 skipped), repository typecheck, ESLint, Prettier, and the complete no-AK integration gate (13 files, 146 tests). The generated evaluation report was removed from the PR and retained only in the ignored local review-artifact directory.

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

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

Not explored to full depth (tool budget reached): "agent 4": none — no check was cut short..

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.

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

  • integration-tests/chat-transcript-contract.test.ts:382 — [review] forbidden-field scan has no positive control on the schema walker
  • integration-tests/chat-transcript-contract.test.ts:294 — [review] fail-first gate alarm fires with a context-free assertion message
  • integration-tests/chat-transcript-contract.test.ts:415 — [review] hash-lock assertion does not name the drifted fixture file
  • integration-tests/chat-transcript-contract.test.ts:505 — [probe] planted canary raw payload is pinned by no assertion
中文说明

⚠️ 已从批准降级为评论:CI still running。 已审查。

未探索到全部深度(达到工具调用预算):"agent 4"none — no check was cut short.

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

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

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline.

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.

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

  • integration-tests/fixtures/chat-transcript-contract/v1/schema/export-transcript-document-v1.schema.json:337 — [review] errorKind 16-entry enum duplicated verbatim across statusBlock and errorBlock
  • integration-tests/chat-transcript-contract.test.ts:271 — [probe] probeIdentity's 'Missing complete identity probe block' fail-closed branch has no test
  • integration-tests/chat-transcript-contract.test.ts:294 — [review] fail-first tripwire expect carries no diagnostic message for the MR2 gate-flip failure
  • integration-tests/chat-transcript-contract.test.ts:273 — [probe] probe compares block IDs only across reductions while design doc §7.5 claims provenance is compared
  • docs/design/web-shell/chat-transcript-contract-prevalidation.md:670 — [probe] §12.1 MR1 whitelist omits the six production/CI files this diff actually changes
中文说明

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

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

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

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

Comment thread integration-tests/chat-transcript-contract.test.ts Outdated
Keep transcript contract prevalidation at the evidence level it can
actually prove. The previous source-text assertion could both reject
equivalent formatting and pass unreachable packaging code.

- Remove the Desktop script parser and its false behavioral claim
- Mark installed-artifact verification as deferred to Desktop smoke tests
- Clarify MR1 matrix, CI wiring, and provenance evidence boundaries
- Refresh the hash-locked capability matrix fixture

Note: This does not change Web Shell or Desktop production behavior.
@water-in-stone

Copy link
Copy Markdown
Collaborator Author

Regarding #9388 (comment): @danialzivehdadr, thanks for the comment. I could not identify a connection between the reported phone/upload behavior and this transcript-contract prevalidation PR. Please open a separate issue with the device and OS, exact page or URL, reproduction steps, expected behavior, actual behavior, and any screenshot or log. This PR does not change mobile upload behavior.

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

⚠️ Downgraded from Approve to Comment: CI failing: Desktop Shell (ubuntu-22.04), Capture web-shell visuals (ubuntu-latest, Node 22.x). Reviewed.

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.

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

  • integration-tests/chat-transcript-contract.test.ts:487 — [probe] runtime-raw-compatibility pin uses toMatchObject subset matching; added fields on rawInput/rawOutput go unseen
  • integration-tests/chat-transcript-contract.test.ts:276 — [probe] fail-first gate tripwire assertion carries no diagnostic message
  • integration-tests/chat-transcript-contract.test.ts:397 — [probe] fixture hash-lock assertion does not name the drifted fixture file
  • integration-tests/chat-transcript-contract.test.ts:15 — [probe] web-shell nodenext import closure carries no marker of the dual module-resolution regime
中文说明

⚠️ 已从批准降级为评论:CI failing: Desktop Shell (ubuntu-22.04), Capture web-shell visuals (ubuntu-latest, Node 22.x)。 已审查。

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

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

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

@water-in-stone

Copy link
Copy Markdown
Collaborator Author

Follow-up on the round-4 review's CI downgrade:

  • The review ledger reported findings: []; its four listed items were explicitly deferred as non-blocking and not requested in this round.
  • The two checks described as failing were actually cancelled before reaching PR behavior: Desktop Shell stopped during Linux dependency installation, and Web-shell Visuals stopped during Playwright installation.
  • I reran both cancelled jobs. Qwen Code CI attempt 2 passed, including Linux Desktop compilation/tests and desktop release tests. Web-shell Visuals attempt 2 also passed, including PR-head and merge-base capture, comparison, and artifact upload.
  • All 19 review threads are resolved. The branch remains at f055ab3c522ff5f69d2de531aac078724fe16d4c, which already matches the pushed PR head; no new source defect was found, so no empty commit was created.

@qwen-code /review

@water-in-stone

Copy link
Copy Markdown
Collaborator Author

@qwen-code /review

@github-actions

Copy link
Copy Markdown
Contributor

Qwen Code review request accepted. Review is queued in workflow run.

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

No blocking issues. LGTM! ✅

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.

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

  • integration-tests/chat-transcript-contract.test.ts:343 — [probe] Lock test pins only a hand-picked subset of the frozen schema's security semantics; unpinned widening vectors pass once MR2 legitimately rotates the fixture hashes
  • integration-tests/chat-transcript-contract.test.ts:276 — [probe] Fail-first gate pivot is a bare unnamed assertion; the good-news failure carries no breadcrumb to the documented gate-flip procedure
  • integration-tests/chat-transcript-contract.test.ts:531 — [probe] Set-valued gate evidence is locked with order-sensitive equality, making reducer block-emission order an implicit contract term
  • docs/design/web-shell/chat-transcript-contract-prevalidation.md:356 — [review] §9.2 still claims MR1 establishes Desktop baseline evidence via source-wiring assertions, which the final commit removed
  • integration-tests/fixtures/chat-transcript-contract/v1/schema/export-transcript-document-v1.schema.json:172 — [probe] Design doc §10.5 promises charset-AND-length constraints for opaque IDs; the frozen schema encodes only length
  • integration-tests/fixtures/chat-transcript-contract/v1/capability-matrix.md:15 — [review] Hash-locked capability matrix names the future VS Code path a 'shared ChatPanel contract', terminology the design doc explicitly supersedes
中文说明

无阻断问题。LGTM!✅

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

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

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

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the design doc and the code changes (types.ts, toolClassification.ts, todos.ts, transcriptToMessages adapter).

Approve. This is MR1 (contract prevalidation) of a two-MR plan; the design doc is a single authoritative source with an explicit fail-closed gate (overall: "fail") and no production type changes yet. The one production-code change I verified — the write_file diff test ensuring the complete diff is kept when a preview is also present — is a correctness fix in the transcript adapter, not a behavior change.

0 unresolved threads, REVIEW_REQUIRED.

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

The ordinal-ID failure still looks real, but I found one direct-daemon fixture mismatch inline. Non-blocking ponytail follow-up for later cleanup: Git already locks fixture bytes, so the manifest schema/hash harness can go, as can the 57-line self-test of the private probe helper. The broader MR2 schema/matrix deferral is already captured in the triage thread.

@@ -0,0 +1,5 @@
{"id":10,"v":1,"type":"session_update","data":{"update":{"sessionUpdate":"user_message_chunk","content":{"type":"text","text":"Inspect the contract"}}}}

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.

[P2] Exercise producer-shaped direct-daemon frames

This fixture wraps the ACP payloads but drops the provenance emitted by the named direct-daemon path. Live BridgeClient.prepareSessionUpdateFrames() spreads activePromptId onto session_update frames (and the user echo carries promptId), while persisted replay stamps _meta.qwenTranscript.sourceRecordIds; normalizeDaemonEvent() preserves both. Because probeIdentity() treats either field as native text identity, the frozen directDaemon.missingNativeTextIdentity result is produced only by these synthetic frames.

The ordinal block IDs still drift, so the overall FAIL can stay. Could we make this fixture producer-shaped and update the direct provenance expectation? If the intended blocker is missing stable segment identity, the probe should check that explicitly instead of defining promptId/sourceRecordIds as sufficient and then omitting them here.

@yiliang114
yiliang114 added this pull request to the merge queue Aug 19, 2026
Merged via the queue into QwenLM:main with commit 5003ab3 Aug 19, 2026
106 of 108 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.15.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants