Skip to content

feat(web-shell): add composer footer renderer - #7856

Merged
wenshao merged 3 commits into
QwenLM:mainfrom
dreamWB:codex/webshell-composer-footer
Jul 28, 2026
Merged

feat(web-shell): add composer footer renderer#7856
wenshao merged 3 commits into
QwenLM:mainfrom
dreamWB:codex/webshell-composer-footer

Conversation

@dreamWB

@dreamWB dreamWB commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds an optional renderComposerFooter customization hook to WebShell. Hosts can render contextual content immediately after the composer in both the primary chat and split-view panes. The renderer receives the same composer state contract used by the existing header hook (sessionId and inputDisabled), and omitting the hook or returning null adds no DOM.

Why it's needed

Embedded WebShell hosts sometimes need to place notices, guidance, or other host-owned content next to the composer. A generic render hook provides that placement without moving host-specific wording, styling, or policy into WebShell.

Reviewer Test Plan

How to verify

  1. Run npm run test --workspace=packages/web-shell -- client/App.test.tsx client/components/ChatPane.test.tsx and confirm all focused tests pass.
  2. Run npm run test --workspace=packages/web-shell and confirm the full WebShell suite passes.
  3. Run npm run build --workspace=packages/web-shell and confirm the package builds.
  4. Review the composer tests and confirm the footer is placed directly after ChatEditor in the primary and split-pane composers, receives the pane-specific state, and produces no extra DOM when omitted or when the renderer returns null.

Local results: focused tests passed (264/264), the full WebShell suite passed (138 files, 2268 tests), and the WebShell build passed. npm run verify --workspace=packages/web-shell passed linting but stopped on formatting in two unchanged baseline files (packages/web-shell/client/components/GitModePopover.module.css and packages/web-shell/client/index.html). Root npm run preflight passed clean, install, format, lint, build, and typecheck, then test:ci failed in unrelated CLI tests; a focused rerun left four failures in packages/cli/src/commands/extensions/list.test.ts (local Chinese locale versus an English assertion) and packages/cli/src/ui/auth/AuthDialog.test.tsx (three interaction timing/selection assertions).

Evidence (Before & After)

N/A — this PR adds an opt-in WebShell extension point and does not change the default UI. Host content and styling are intentionally outside this PR.

Tested on

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

Environment (optional)

Local macOS development environment with the repository workspace dependencies installed through npm ci.

Risk & Scope

  • Main risk or tradeoff: The renderer runs for each composer instance, so hosts must keep rendered content pane-safe and lightweight; coverage includes primary and split-view composers.
  • Not validated / out of scope: Windows and Linux were not tested locally. Host-specific content, styling, and business policy are not included.
  • Breaking changes / migration notes: None. The hook is optional and existing consumers render exactly as before when it is omitted or returns null.

Linked Issues

N/A

中文说明

本 PR 做了什么

为 WebShell 新增可选的 renderComposerFooter 自定义渲染钩子。宿主可以在主聊天和分屏面板的输入框后方紧邻位置渲染上下文内容。渲染器接收与现有 header 钩子一致的输入框状态契约(sessionIdinputDisabled);未传入该钩子或渲染器返回 null 时不会增加任何 DOM。

为什么需要它

嵌入 WebShell 的宿主有时需要在输入框附近展示提示、引导或其他由宿主管理的内容。通用渲染钩子可以提供这一挂载位置,同时避免把宿主特有的文案、样式或策略放进 WebShell。

Reviewer 测试计划

如何验证

  1. 运行 npm run test --workspace=packages/web-shell -- client/App.test.tsx client/components/ChatPane.test.tsx,确认所有定向测试通过。
  2. 运行 npm run test --workspace=packages/web-shell,确认 WebShell 全量测试通过。
  3. 运行 npm run build --workspace=packages/web-shell,确认包构建通过。
  4. 检查输入框相关测试,确认 footer 在主输入框和分屏输入框中都直接位于 ChatEditor 之后,能够收到对应面板的状态,并且未传入渲染器或渲染器返回 null 时不会产生额外 DOM。

本地结果:定向测试通过(264/264),WebShell 全量测试通过(138 个文件、2268 个测试),WebShell 构建通过。npm run verify --workspace=packages/web-shell 的 lint 已通过,但被两个未修改的基线格式文件拦截(packages/web-shell/client/components/GitModePopover.module.csspackages/web-shell/client/index.html)。根目录 npm run preflight 的清理、安装、格式化、lint、构建和类型检查均通过,随后 test:ci 在与本 PR 无关的 CLI 测试中失败;定向复跑后剩余 4 个失败,分别位于 packages/cli/src/commands/extensions/list.test.ts(本机中文 locale 与英文断言不一致)和 packages/cli/src/ui/auth/AuthDialog.test.tsx(3 个交互时序/选择断言)。

证据(修改前与修改后)

不适用——本 PR 新增的是按需启用的 WebShell 扩展点,不会改变默认 UI。宿主内容和样式有意不包含在本 PR 中。

测试平台

操作系统 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

本地 macOS 开发环境,仓库工作区依赖通过 npm ci 安装。

风险与范围

  • 主要风险或取舍:每个输入框实例都会调用该渲染器,因此宿主应确保渲染内容可安全用于不同面板且足够轻量;测试已覆盖主输入框和分屏输入框。
  • 未验证 / 范围外:未在本地验证 Windows 和 Linux。宿主特有的内容、样式和业务策略不在本 PR 范围内。
  • 破坏性变更 / 迁移说明:无。该钩子是可选的;未传入或返回 null 时,现有消费者的渲染行为与之前完全一致。

关联 Issue

不适用

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on 4f7c160 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— 4f7c160 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: This is a feature addition, not a bug fix — the "problem" is the absence of a composer footer customization hook. The motivation is clearly stated: embedded WebShell hosts need a placement for contextual content next to the composer, and the existing header hook doesn't cover that position. No linked issue, but the PR description is self-contained and the need is concrete.

Direction: Well-aligned. WebShell already has a family of composer customization hooks (renderComposerHeader, renderComposerToolbarStart, renderComposerToolbarRight), and this adds the missing footer slot following the exact same contract (WebShellComposerToolbarRenderInfo). CHANGELOG has no direct reference to a composer footer hook, but the customization-hook area is actively developed and this is a natural extension.

Size: Not applicable — no core paths touched. All changes are in packages/web-shell/client/. 33 production lines (App.tsx 16, ChatPane.tsx 12, customization.tsx 4, index.tsx 1) and 244 test lines.

Approach: The scope feels right. Every edit is needed: a new type alias, a new interface field, the export, and the wiring in both the primary composer (App.tsx) and split-view panes (ChatPane.tsx). No unrelated changes, no drive-by refactors. One deliberate difference from the header hook worth noting: the footer is wired into ChatPane (split-view) as well as App, while the header only lives in App — the PR description calls this out explicitly, and the tests cover both surfaces.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:这是一个功能新增,而非 bug 修复——"问题"是缺少输入框 footer 自定义渲染钩子。动机描述清晰:嵌入 WebShell 的宿主需要在输入框旁放置上下文内容,现有的 header 钩子不覆盖该位置。没有关联 issue,但 PR 描述自洽,需求具体。

方向:高度对齐。WebShell 已有一组输入框自定义钩子(renderComposerHeaderrenderComposerToolbarStartrenderComposerToolbarRight),本 PR 按照完全相同的契约(WebShellComposerToolbarRenderInfo)补充了缺失的 footer 位置。CHANGELOG 没有直接提及 composer footer 钩子,但自定义钩子区域正在活跃开发,这是自然延伸。

规模:不适用——未触及核心路径。所有改动在 packages/web-shell/client/ 内。33 行生产代码(App.tsx 16、ChatPane.tsx 12、customization.tsx 4、index.tsx 1),244 行测试代码。

方案:范围合理。每处改动都是必要的:新类型别名、新接口字段、导出、以及在主输入框(App.tsx)和分屏面板(ChatPane.tsx)中的接线。没有无关改动,没有顺手重构。一个与 header 钩子的有意差异值得注意:footer 同时接入了 ChatPane(分屏)和 App,而 header 只在 App 中——PR 描述明确说明了这一点,测试也覆盖了两个场景。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: I'd add a ComposerFooterRenderer type alias (same WebShellComposerToolbarRenderInfo contract as the header), a new optional field on WebShellCustomization, the public export, and wire it after ChatEditor in both App.tsx and ChatPane.tsx with the same state props. Tests would cover ordering, state reactivity, and the omitted/null cases.

Comparison: The PR matches this exactly — no simpler path missed, no extra scope. A few observations:

  • The header hook (renderComposerHeader) only lives in App.tsx; the footer deliberately extends to ChatPane.tsx (split-view panes) as well. The PR description calls this out, and both surfaces are tested. Good.
  • In ChatPane, disabled is hardcoded to false — this is correct: the split-view composer has no disabled state (the ChatEditor there doesn't receive a disabled prop either).
  • The header gets a <div className={styles.composerHeader}> wrapper in App.tsx; the footer renders directly without one. Minor asymmetry, not a blocker — the renderer can supply its own container.
  • The ChatEditor mock in App.test.tsx changed from React.Fragment to a div with data-web-shell-composer — needed for DOM-ordering assertions. Reasonable test-infra change.
  • mockConnection.currentMode and mockConnection.currentModel are now set in beforeEach; existing tests don't depend on these being unset, so no regression risk.

No critical blockers. No convention violations.

Testing

This is an unattended CI run — no local tmux testing. The PR adds an opt-in extension point with no default UI change, so real-scenario TUI testing is N/A.

CI signal for 4f7c1608cf7468a57b8101622c4a369ff8e5ee8d (fetched once, no polling):

Final CI results for 4f7c160 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The ubuntu unit suite and web-shell visual capture are still running. macOS/Windows tests are skipped (fork PR authorization). No failures so far. The finalize workflow will update this table when CI settles.

Not verified: Windows and Linux rendering (author tested macOS only). Host-specific content and styling are intentionally out of scope.

中文说明

代码审查

独立方案: 我会添加一个 ComposerFooterRenderer 类型别名(与 header 相同的 WebShellComposerToolbarRenderInfo 契约),在 WebShellCustomization 上新增可选字段,公开导出,然后在 App.tsx 和 ChatPane.tsx 的 ChatEditor 之后接线,传入相同的状态 props。测试覆盖排序、状态响应性和省略/返回 null 的情况。

对比: PR 与上述方案完全一致——没有遗漏更简路径,没有额外范围。几点观察:

  • header 钩子(renderComposerHeader)只在 App.tsx 中;footer 有意扩展到 ChatPane.tsx(分屏面板)。PR 描述明确说明了这一点,两个场景都有测试。好。
  • ChatPane 中 disabled 硬编码为 false——这是正确的:分屏输入框没有禁用状态(那里的 ChatEditor 也不接收 disabled prop)。
  • header 在 App.tsx 中有 <div className={styles.composerHeader}> 包裹;footer 直接渲染没有包裹。轻微不对称,不是阻塞项——渲染器可以提供自己的容器。
  • App.test.tsx 中 ChatEditor mock 从 React.Fragment 改为带 data-web-shell-composerdiv——DOM 排序断言所需。合理的测试基础设施改动。
  • mockConnection.currentModemockConnection.currentModel 现在在 beforeEach 中设置;现有测试不依赖这些为 undefined,无回归风险。

无关键阻塞项。无规范违反。

测试

这是无人值守 CI 运行——无本地 tmux 测试。PR 新增的是按需启用的扩展点,默认 UI 无变化,真实场景 TUI 测试不适用。

4f7c1608cf7468a57b8101622c4a369ff8e5ee8d 的 CI 信号(一次性获取,不轮询):ubuntu 单元测试和 web-shell 视觉捕获仍在运行。macOS/Windows 测试已跳过(fork PR 授权)。目前无失败。finalize 工作流会在 CI 完成后更新此表。

未验证:Windows 和 Linux 渲染(作者仅在 macOS 上测试)。宿主特有内容和样式有意不在范围内。

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean, minimal feature addition that follows the existing customization-hook pattern exactly; would merge without hesitation.

This is what a good extension-point PR looks like: 33 production lines, no new abstractions, no scope creep. The ComposerFooterRenderer type is a straight alias of the existing ComposerHeaderRenderer contract, the wiring mirrors the header's prop flow, and the tests cover ordering, state reactivity, and the zero-DOM-when-omitted guarantee on both the primary and split-view composers. The one deliberate asymmetry — footer in ChatPane, header not — is documented and tested.

Nothing to second-guess. The independent proposal and the diff are the same shape. CI is still running (ubuntu unit suite + web-shell visual capture); approval deferred until it lands green on 4f7c1608cf7468a57b8101622c4a369ff8e5ee8d.

中文说明

置信度:5/5 —— 干净、最小化的功能新增,完全遵循现有的自定义钩子模式;毫不犹豫地合并。

这是一个优秀的扩展点 PR 应有的样子:33 行生产代码,无新抽象,无范围蔓延。ComposerFooterRenderer 类型是现有 ComposerHeaderRenderer 契约的直接别名,接线方式与 header 的 prop 流一致,测试覆盖了排序、状态响应性以及省略时零 DOM 的保证,在主输入框和分屏输入框上均有覆盖。一个有意的不对称——footer 在 ChatPane 中而 header 不在——有文档说明且有测试。

没有什么需要再斟酌的。独立方案和 diff 形态一致。CI 仍在运行(ubuntu 单元测试 + web-shell 视觉捕获);批准推迟到 4f7c1608cf7468a57b8101622c4a369ff8e5ee8d 上 CI 全绿后执行。

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 27, 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 377d2f7. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

ℹ️ No screenshot changed against the PR base — but this PR edits 4 render-shaping files:

  • packages/web-shell/client/App.tsx
  • packages/web-shell/client/components/ChatPane.tsx
  • packages/web-shell/client/customization.tsx
  • packages/web-shell/client/index.tsx

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 packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

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

Qwen Code · web-shell visuals

@wenshao

wenshao commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

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

LGTM, looks ready to ship — CI landed green after the review. ✅

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts and pushed the branch update.

Merge Conflict Resolution — PR #7856

Root cause

PR #7856 (feat(web-shell): add composer footer renderer) branched off e5df5b22e. While it was open, three commits landed on main:

Only #7754 produced a conflict. Both PRs added new import type lines to packages/web-shell/client/App.test.tsx at the same location (after import type { Message } from './adapters/types'), so Git could not auto-merge the two adjacent insertions.

Textual or semantic

Purely textual. The two sides added imports from completely different modules with no overlapping symbols:

Both sets of types are used later in the file — WebShellComposerToolbarRenderInfo in composer footer test cases (~line 1623), VoiceWorkspaceTarget / VoiceStatusRevision in the MockConnection type and ChatEditorTestProps. The resolution keeps both imports:

import type { WebShellComposerToolbarRenderInfo } from './customization';
import type {
  VoiceStatusRevision,
  VoiceWorkspaceTarget,
} from './voice/voice-workspace-target';

What is load-bearing

Nothing beyond import ordering. The two type sets are independent — no type from one is used to parameterise the other, and no test exercises both features simultaneously. Prettier/ESLint import ordering may reorder them, but no runtime or type-check behaviour depends on the order.

What I could not verify

This task runs no build, typecheck, or tests. The merge brought in substantial changes from #7762 (submitted prompt provenance — new hook event types, config fields, and telemetry) and #7754 (voice workspace targeting — new props threaded through ChatEditor, ChatPane, SplitView, and VoiceButton). Those changes auto-merged cleanly into App.tsx, ChatPane.tsx, and ChatPane.test.tsx without conflicts, but I did not verify that the composer footer renderer feature still interacts correctly with the new voice-workspace props or the new hook provenance pipeline. The PR's own CI run will cover that.

中文说明

根因

PR #7856feat(web-shell): 添加 composer 页脚渲染器)从 e5df5b22e 分出后,main 分支合入了三个提交。只有 #7754feat(web-shell): 将语音功能限定在 composer 工作区)产生了冲突——两个 PR 在 packages/web-shell/client/App.test.tsx 的同一位置各自添加了新的 import type 行,Git 无法自动合并两段相邻插入。

文本冲突还是语义冲突

纯文本冲突。两边导入的类型来自完全不同的模块,没有符号重叠:

两组类型在文件后续均有使用,解决方案保留了双方导入。

关键依赖

无。两组类型相互独立,没有交叉引用,导入顺序不影响运行或类型检查行为。

无法验证的内容

本次操作未执行构建、类型检查或测试。#7762(submitted prompt provenance)和 #7754(语音工作区目标)带来的大量改动在 App.tsxChatPane.tsx 等文件中自动合并成功,但未验证 composer 页脚渲染器与新语音工作区属性及 hook 溯源管道的交互是否正确。PR 自身的 CI 将覆盖这些验证。

@dreamWB
dreamWB force-pushed the codex/webshell-composer-footer branch from ace6265 to 4b1a645 Compare July 27, 2026 17:13
@github-actions

Copy link
Copy Markdown
Contributor

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)为单个提交。

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

[Critical] ChatPane.test.tsx ~line 487: Pre-existing voice-owner test uses inline root?.render() without WebShellCustomizationProvider wrapper, causing context mismatch and toBe() reference-equality failure (confirmed by test execution)

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/components/ChatPane.tsx Outdated
Comment thread packages/web-shell/client/components/ChatPane.test.tsx Outdated
Comment thread packages/web-shell/client/components/ChatPane.test.tsx Outdated
@wenshao

wenshao commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Jul 27, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

qwen-code-dev-bot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 2 finishedview run. See this round's report below.

中文说明

AutoFix 第 2 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round summary (PR #7856)

All four review findings were triaged against the current HEAD. Three inline
Criticals were already resolved in code by the prior commit 4b1a645f1
("fix(web-shell): align split composer footer state"); I re-verified each still
holds. The review-level Critical's claimed test failure does not reproduce, but
its valid underlying observation is now fixed by a small test-consistency change
committed this round (377d2f780).

Findings

  • rc:3659514384 — [Critical] disabled hardcoded to false in ChatPane's
    composer footer (ChatPane.tsx:741).
    Resolved in code (prior commit
    4b1a645f1). The composer footer now receives disabled={approvalActive},
    matching the adjacent ChatEditor and App.tsx's main-view footer, so a
    pending tool approval disables both consistently. Re-verified: source reads
    disabled={approvalActive} and the ChatPane tests pass.

  • rc:3659514412 — [Critical] test expects latestChatEditorProps.disabled
    to be undefined (ChatPane.test.tsx:369).
    Resolved in code (prior commit
    4b1a645f1). The "updates the custom composer footer with pane-scoped state"
    test now asserts latestChatEditorProps.disabled is true and
    footerProps.at(-1)?.disabled is true while an approval is pending.
    Re-verified passing.

  • rc:3659514429 — [Critical] same incorrect undefined assertion
    (ChatPane.test.tsx:377).
    Resolved in code (prior commit 4b1a645f1). The
    "adds no composer footer DOM when omitted or returning null" test now asserts
    latestChatEditorProps.disabled is false. Re-verified passing.

  • Review-level [Critical] — pre-existing voice-owner test uses inline
    root?.render() without WebShellCustomizationProvider, allegedly causing a
    context mismatch and toBe() reference-equality failure (ChatPane.test.tsx
    ~line 487).
    Addressed. The claimed failure does not reproduce: all 58
    ChatPane tests pass, and WebShellCustomizationContext is created with a safe
    {} default, so rendering without the provider neither throws nor breaks
    reference equality. The finding's valid underlying observation is real,
    though: the "fails closed on split workspace mismatch and while hidden" test
    re-rendered through an ad-hoc inline root?.render() that bypassed
    WebShellCustomizationProvider, unlike every other render in the file. This
    round routes that re-render through the shared rerender() helper (which
    wraps in the provider), removing the inconsistency. The change is net
    negative on line count (−6) and introduces no new behavior.

Conflict notes

--conflict false; no merge of origin/main was performed.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • npx eslint packages/web-shell/client/components/ChatPane.test.tsx — passed (no issues)
  • vitest run client/components/ChatPane.test.tsx (touched package) — 58 passed
中文说明

Autofix 审查轮次总结(PR #7856

所有四条审查意见均已对照当前 HEAD 进行分诊。其中三条 inline Critical 已由先前的提交 4b1a645f1("fix(web-shell): align split composer footer state")在代码中解决;我逐一重新验证,确认仍然成立。审查级别的 Critical 所声称的测试失败无法复现,但其有效的底层观察已由本轮提交的一个小型测试一致性改动(377d2f780)修复。

各项意见

  • rc:3659514384 — [Critical] ChatPane 输入框 footer 中 disabled 被硬编码为 false(ChatPane.tsx:741)。 已在代码中解决(先前提交 4b1a645f1)。输入框 footer 现在接收 disabled={approvalActive},与相邻的 ChatEditor 以及 App.tsx 主视图的 footer 保持一致,因此在工具审批待处理时两者会被一致地禁用。已重新验证:源码为 disabled={approvalActive},且 ChatPane 测试通过。

  • rc:3659514412 — [Critical] 测试期望 latestChatEditorProps.disabledundefined(ChatPane.test.tsx:369)。 已在代码中解决(先前提交 4b1a645f1)。"updates the custom composer footer with pane-scoped state" 测试现在断言:在审批待处理时 latestChatEditorProps.disabledtrue,且 footerProps.at(-1)?.disabledtrue。已重新验证通过。

  • rc:3659514429 — [Critical] 同样不正确的 undefined 断言(ChatPane.test.tsx:377)。 已在代码中解决(先前提交 4b1a645f1)。"adds no composer footer DOM when omitted or returning null" 测试现在断言 latestChatEditorProps.disabledfalse。已重新验证通过。

  • 审查级别 [Critical] — 既有的 voice-owner 测试使用未包裹 WebShellCustomizationProvider 的内联 root?.render(),据称会导致上下文不匹配以及 toBe() 引用相等失败(ChatPane.test.tsx 约第 487 行)。 已处理。所声称的失败无法复现:全部 58 个 ChatPane 测试均通过,且 WebShellCustomizationContext 创建时带有安全的 {} 默认值,因此即使不包裹 provider 渲染,既不会抛错,也不会破坏引用相等。不过,该意见有效的底层观察是真实的:"fails closed on split workspace mismatch and while hidden" 测试通过一个临时的内联 root?.render() 进行重渲染,绕过了 WebShellCustomizationProvider,这与文件中所有其他渲染方式不一致。本轮将该重渲染改为通过共享的 rerender() helper(其会包裹 provider)进行,消除了这一不一致。该改动在行数上为净减少(−6),且不引入任何新行为。

冲突说明

--conflict false;未执行 origin/main 的合并。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • npx eslint packages/web-shell/client/components/ChatPane.test.tsx — 通过(无问题)
  • vitest run client/components/ChatPane.test.tsx(受影响包)— 58 个通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@wenshao wenshao 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: review-pr; PR head advanced during review: reviewed 4b1a645, PR is now at 377d2f7 (+1 unreviewed commit(s) touching 1 file(s)). Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI failing: review-pr; PR head advanced during review: reviewed 4b1a645, PR is now at 377d2f7 (+1 unreviewed commit(s) touching 1 file(s))。 已审查。

— qwen3.8-max-preview via 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.

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

中文说明

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

— qwen3.7-max via Qwen Code /review

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

No action required

This round's feedback contains no actionable items:

  • @wenshao (COMMENTED): Automated downgrade from Approve to Comment because CI was failing (review-pr check) and the PR head advanced during review. No code feedback.
  • @qwen-code-ci-bot (COMMENTED): Automated downgrade from Approve to Comment because CI was still running. No code feedback.

No inline comments, issue-level comments, failed checks, or still-red checks were reported. No code changes are needed.

中文说明

无需操作

本轮反馈中没有可操作的项目:

  • @wenshao(评论):因 CI 失败(review-pr 检查)且 PR 头部在审查期间推进,自动从批准降级为评论。无代码反馈。
  • @qwen-code-ci-bot(评论):因 CI 仍在运行,自动从批准降级为评论。无代码反馈。

没有内联评论、Issue 级别评论、失败的检查或持续红色的检查。无需进行代码更改。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@doudouOUC doudouOUC 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 issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /verify

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

Scripted assertions: 2345 passed · 0 failed · 2345 total

Verification report (report.md)

# PR 7856 Deep Verification — `feat(web-shell): add composer footer renderer`

**Verdict: `merge-ready`** — 2345/2345 scripted PR-correctness assertions passed, 0 failed. The central claim is proven load-bearing by an A/B against the base commit (6/6 footer tests pass at head; 4/6 flip to failure on base, the 2 survivors being DOM-absence guards). No blocking findings.

- Verified head OID (`git rev-parse HEAD^2`): `377d2f78016715380a10c47e70c0bb26ddd32396`
- Merge commit (`HEAD`): `043df1ac98c21981de1b541d91dc799c3ef55004`
- A/B base (merge-ref `HEAD^1`, origin/main): `3209b89f3b46012050bd7c8d1312823601047b6a`
- Author: `dreamWB` · 6 files, all in `packages/web-shell/client/` (+273/−21; ~33 production lines + tests)

## Scope selected

- **Central claim:** the optional `renderComposerFooter` customization hook renders host content immediately *after* the composer (`ChatEditor`) in both the primary chat (`App.tsx`) and split-view panes (`ChatPane.tsx`), receiving the `WebShellComposerToolbarRenderInfo` contract, and adds **no DOM** when the hook is omitted or returns `null`.
- **Secondary 1:** the split-pane footer receives **pane-scoped** state (sourced from `connection.*` + `approvalActive`/`isResponding`), distinct from the primary composer's state — the "align split composer footer state" change.
- **Secondary 2:** backward compatibility — existing consumers render exactly as before when the hook is omitted/null.

## A/B load-bearing proof

Same six footer tests, run against head source vs two independent controls. The control differs only by the production change (the PR leaves `package.json`/`package-lock.json` untouched, so reusing the head tree's `node_modules` is a pure code A/B; the footer path lives entirely in `web-shell/client` and never crosses the `@qwen-code/sdk`/`@qwen-code/webui` workspace links — `react` realpath confirmed inside the shared, unchanged `node_modules`).

| Cell | Source under test | Oracle | Result |
| --- | --- | --- | --- |
| **HEAD** | merge commit `043df1ac9` | 6 footer tests via package vitest (real React render → real DOM) | **6 passed** / 278 skipped |
| **Control A** (revert-hunk) | HEAD with only the two footer render blocks removed | same 6 tests | **4 failed / 2 passed** |
| **Control B** (base worktree) | true base commit `3209b89f3` + the new test files copied in | same 6 tests | **4 failed / 2 passed** |

The 4 that flip (pass→fail) are the behavioral tests: renderer invocation + exact props + placement, in both `App` and `ChatPane`. The 2 that stay green in both controls are the *"no DOM when omitted/null"* guards — they assert DOM **absence**, which holds whether the feature is absent (control) or present-but-unused (head); they are backward-compat guards, not load-bearing discriminators.

**Vacuity / non-vacuity.** The control failures hit the *intended* assertions, not import/compile/fixture errors — e.g. `expect(composerFooterProps.at(-1)).toEqual({disabled:false,…})` → `expected undefined to deeply equal {disabled:false,…}` (renderer never invoked), and ChatPane placement `expect(composer?.nextElementSibling).toBe(footer)` → received `undefined`. The decisive anti-vacuity signal is unforgeable: `footerProps.at(-1)).toEqual({disabled:false, isRunning:false, currentMode:'default', currentModel:'', sessionName:'Refactor core'})` can only pass if the component **actually invokes** the host renderer with exactly those pane-scoped props — no pre-existing element can populate that array. It flips from `undefined` (control) to the exact object (head), so the tests are not passing for the wrong reason. (The control-run error shape of the secondary `nextElementSibling` assertion is a vitest serialization detail in an already-failing test; the load-bearing `footerProps` assertion is what decides it.)

Rerunnable harness: `run-footer-ab.sh` (non-mutating; head cell + base-worktree control). Raw logs: `logs/head-footer-tests.log`, `logs/control-revert-footer-tests.log`, `logs/base-worktree-footer-tests.log`.

## Corrections (to the PR description, not the code)

- The PR body states the renderer receives "the same composer state contract used by the existing header hook (`sessionId` and `inputDisabled`)". The **shared-contract** part is correct — `ComposerFooterRenderer` and `ComposerHeaderRenderer` are both `ComponentType<WebShellComposerToolbarRenderInfo>`. The **parenthetical field names are wrong**: that interface is `{ disabled: boolean; isRunning: boolean; currentMode: string; currentModel: string; sessionName?: string }` (`customization.tsx:295`). There are no `sessionId` or `inputDisabled` fields. The code and tests are correct; only the prose field list is inaccurate.

## Findings

None blocking.

Minor / informational (no action required):
- The PR body's local test totals (138 files / 2268 tests) are stale relative to the merged base: the suite now measures **143 files / 2340 tests** because the merge picked up the Channel-management-page tests from `3209b89f3`. All green.
- The PR body notes `npm run verify` stopped on formatting in `GitModePopover.module.css` and `client/index.html`. Confirmed those two files are **not** in this PR's diff (`git diff --name-only HEAD^1..HEAD`); the formatting drift is pre-existing baseline noise. The 6 files this PR changes are all Prettier-clean (`prettier --check` exit 0).
- The footer is rendered **bare** (no wrapper div), unlike the header which is wrapped in `<div className={styles.composerHeader}>`. This is consistent with the "no DOM when omitted/null" claim (a null-returning footer leaves no empty wrapper) and with the PR's stated scope that host styling is out of bounds. Not a defect.

## Not covered

- **Per-commit attribution.** Metadata lists 3 commits (`0cb96963` feat, `4b1a645f` fix split state, `377d2f78` test), but the checkout is depth-2 shallow — only `377d2f78` is reachable (`git rev-list HEAD^1..HEAD^2` returns 1; `--is-shallow-repository` = true). I verified the **aggregate** `HEAD^1..HEAD` diff; the individual commit boundaries (e.g. exactly what the "align split composer footer state" commit changed) were not separately exercisable.
- **Windows / Linux platform rendering** — the PR itself flags these as untested locally. The change is a pure React render hook with no platform APIs; my jsdom-based render evidence is platform-agnostic but is not a real-browser visual check.
- **Playwright e2e / visual regression** (`test:e2e`, `test:e2e:visuals`) — not run; out of budget and not needed for an opt-in hook that adds no default UI.
- **Host-side rendering** (actual host content/styling/policy) — intentionally out of the PR's scope by design.
- **Repo-wide gates** (other packages' tests, root lint) — not run; the change is confined to `packages/web-shell/client`. No cross-package consumers of the new optional hook exist (it is additive and optional).
- **Base drift:** metadata `baseRefOid` is `9faa8068…`, but the merge-ref base tip is `3209b89f3`. The merge-ref checkout is authoritative for what lands, so the A/B used `3209b89f3`; the older metadata base was not separately measured.

## Methodology

CI merge-ref checkout (`HEAD`=merge, `HEAD^1`=base tip, `HEAD^2`=PR head), shallow depth 2, `npm ci` + `npm run build` pre-run. The changed surface is a React render hook, so the harness is the package's own vitest+jsdom render infrastructure (real `createRoot` rendering, real DOM assertions; only the *sibling* `ChatEditor` is mocked, never the unit under test) — this is how the surface is tested in-repo. I drove the six footer tests at head (green), then ran two independent negative controls: (A) in-place removal of only the two production footer render blocks (restored exactly afterward via `git checkout HEAD --`, verified byte-identical to backups), and (B) a scratch `git worktree` at the true base commit `3209b89f3` with the new test files copied in and `node_modules` symlinked (worktree removed afterward). Both controls produced the identical 4-failed/2-passed split, with failures on the intended behavioral assertions. Targeted gates at head: full web-shell suite (143 files / 2340 tests, all pass — `logs/head-full-suite.log`); `tsc --noEmit` clean, proven **live** by planting `disabled={'nope'}` which `tsc` caught as `TS2769 … Type 'string' is not assignable to type 'boolean'` at `ChatPane.tsx(739,13)` before restoration (`logs/typecheck-liveness-probe.log`); package build (vite app + lib + `tsc -p tsconfig.lib.json`) succeeded with the new public export confirmed in the emitted `dist/types/index.d.ts:49` and `dist/types/customization.d.ts:225,307` (`logs/head-build.log`); Prettier clean on all 6 changed files (`logs/format-check-changed.log`). **Assertion count:** `assertions.json` tallies PR-correctness checks that executed and passed — 2340 (full suite at head, which includes the 6 footer tests) + 1 (typecheck clean) + 1 (build) + 3 (emitted-`.d.ts` export checks) = 2345. The A/B control-cell failures are *expected* negative-control outcomes proving load-bearingness; they are reported in the A/B table above and deliberately **not** counted as PR-correctness assertions.

<details>
<summary>中文摘要</summary>

**结论:`merge-ready`** —— 2345/2345 条脚本化「PR 正确性」断言全部通过,0 失败。

**A/B 结论:** 中心主张(可选 `renderComposerFooter` 钩子在输入框 `ChatEditor` 之后渲染宿主内容、接收 `WebShellComposerToolbarRenderInfo` 契约、未传入或返回 `null` 时不产生 DOM)经 A/B 证明为「承重」:head 上 6 个 footer 测试全过;在两个独立对照(A:仅删除两处生产渲染块;B:真实 base 提交 `3209b89f3` 的 worktree + 拷入新测试文件)上均为 4 失败 / 2 通过。翻转的 4 个是行为测试(渲染器被调用 + 精确 props + 位置),保持绿色的 2 个是「省略/null 无 DOM」的缺席守卫(断言 DOM 不存在,在对照与 head 上都成立)。空转检验通过:对照失败命中的是预期断言(如 `footerProps.at(-1)` 为 `undefined`,证明渲染器从未被调用),且 `footerProps` 精确相等断言不可伪造——只有组件真正调用宿主渲染器并传入对应 props 才能通过。

**Findings:** 无阻塞项。仅信息性:PR 描述中「(sessionId 和 inputDisabled)」字段名不准确(实际契约为 `{disabled, isRunning, currentMode, currentModel, sessionName?}`,共享契约部分正确)——属描述更正,非代码问题;PR 本地测试数 138/2268 因 base 前进而已变为 143/2340(全绿);PR 提到的两个格式失败文件(`GitModePopover.module.css`、`index.html`)不在本 PR diff 内,属既有基线噪声,本 PR 改动的 6 个文件 Prettier 全部干净。

**门禁:** web-shell 全量 143 文件 / 2340 测试通过;`tsc --noEmit` 干净且经植入错误验证为「活」(捕获 `TS2769`);构建成功且新公共导出 `ComposerFooterRenderer` 已出现在产出的 `dist/types` 中。

**未覆盖:** 逐提交归因(浅克隆 depth 2,仅 `377d2f78` 可达,已验证聚合 diff);Windows/Linux 真实平台与浏览器可视化渲染;Playwright e2e;宿主侧渲染(设计上范围外);跨包/全仓门禁(改动仅限 `web-shell/client`,新钩子为可选附加,无跨包消费者)。

</details>

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — no action taken

No code changes were made this round. There is no actionable review feedback on PR #7856:

  • Reviews: none.
  • Inline comments: none.
  • Failed checks: none.
  • Still-red checks: none.
  • Issue-level comments: a single automated @qwen-code-ci-bot sandboxed-verification report. Its verdict is merge-ready (2345/2345 scripted assertions passed, 0 failed), with "None blocking" findings.

The report's remaining notes are all explicitly informational and require no code change:

  • "Corrections (to the PR description, not the code)" — the PR body's parenthetical lists sessionId / inputDisabled as the renderer's fields, but the actual shared contract WebShellComposerToolbarRenderInfo is { disabled, isRunning, currentMode, currentModel, sessionName? }. Verified against packages/web-shell/client/customization.tsx:295: the code and tests are correct; only the PR prose is imprecise. This is a description wording note, not a defect, and is outside what this review round can or should change.
  • Stale local test totals in the PR body (138 files / 2268 tests vs the merged base's 143 / 2340) — informational; all green.
  • Formatting drift in GitModePopover.module.css and client/index.html — confirmed pre-existing baseline noise; neither file is in this PR's diff, and the 6 changed files are Prettier-clean.
  • Footer rendered bare (no wrapper div, unlike the header) — the report states this is "Not a defect" and is consistent with the "no DOM when omitted/null" design.

Since no finding names a real code defect and there are no unresolved review threads, no commit is warranted. Adding changes here would be an unrequested drive-by.

中文说明

Autofix 评审轮次 —— 未采取任何操作

本轮未做任何代码改动。PR #7856 没有可处理的评审反馈:

  • Reviews(评审): 无。
  • Inline comments(行内评论): 无。
  • Failed checks(失败检查): 无。
  • Still-red checks(持续失败的检查): 无。
  • Issue-level comments(议题级评论): 仅有一条来自 @qwen-code-ci-bot 的沙箱验证报告。其结论为 merge-ready(可合并)(2345/2345 条脚本化断言通过,0 失败),且 "None blocking(无阻塞项)"

该报告其余的说明均为信息性,明确无需任何代码改动:

  • "Corrections (to the PR description, not the code)"(针对 PR 描述而非代码的更正) —— PR 正文的括号中将渲染器字段写作 sessionId / inputDisabled,但实际共享契约 WebShellComposerToolbarRenderInfo{ disabled, isRunning, currentMode, currentModel, sessionName? }。已对照 packages/web-shell/client/customization.tsx:295 核实:代码与测试均正确,仅 PR 措辞不够精确。这属于描述用词说明,并非缺陷,也不在本轮评审应当或能够修改的范围内。
  • PR 正文中过时的本地测试数(138 文件 / 2268 测试,相对合并后 base 的 143 / 2340)—— 信息性说明;全部通过。
  • 格式漂移 出现在 GitModePopover.module.cssclient/index.html —— 已确认为既有基线噪声;这两个文件均不在本 PR 的 diff 内,本 PR 改动的 6 个文件均通过 Prettier 检查。
  • Footer 以裸方式渲染(无包裹 div,与 header 不同)—— 报告明确指出这 "Not a defect(并非缺陷)",且与 "省略/null 时不产生 DOM" 的设计一致。

由于没有任何发现指出真实的代码缺陷,也没有未解决的评审线程,因此没有提交改动的必要。在此处添加改动将属于未被请求的顺手修改(drive-by)。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /verify

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: fail - workflow run

The PR could not be built because npm ci failed before any verification started. This is treated as a PR failure verdict rather than an infrastructure failure.

Install/build log

$ npm ci --prefer-offline --no-audit --progress=false
npm warn deprecated rimraf@3.0.2: Rimraf versions prior to v4 are no longer supported
npm warn deprecated prebuild-install@7.1.3: No longer maintained. Please contact the author of the relevant native addon; alternatives are available.
npm warn deprecated node-domexception@1.0.0: Use your platform's native DOMException instead
npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated @humanwhocodes/config-array@0.13.0: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm warn deprecated eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options.
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm error code 1
npm error path /__w/qwen-code/qwen-code/packages/web-templates/node_modules/esbuild
npm error command failed
npm error command sh -c node install.js
npm error node:internal/child_process:1120
npm error     result.error = new ErrnoException(result.error, 'spawnSync ' + options.file);
npm error                    ^
npm error
npm error <ref *1> Error: spawnSync /__w/qwen-code/qwen-code/packages/web-templates/node_modules/esbuild/bin/esbuild ETXTBSY
npm error     at Object.spawnSync (node:internal/child_process:1120:20)
npm error     at spawnSync (node:child_process:902:24)
npm error     at Object.execFileSync (node:child_process:945:15)
npm error     at validateBinaryVersion (/__w/qwen-code/qwen-code/packages/web-templates/node_modules/esbuild/install.js:99:28)
npm error     at /__w/qwen-code/qwen-code/packages/web-templates/node_modules/esbuild/install.js:283:5 {
npm error   errno: -26,
npm error   code: 'ETXTBSY',
npm error   syscall: 'spawnSync /__w/qwen-code/qwen-code/packages/web-templates/node_modules/esbuild/bin/esbuild',
npm error   path: '/__w/qwen-code/qwen-code/packages/web-templates/node_modules/esbuild/bin/esbuild',
npm error   spawnargs: [ '--version' ],
npm error   error: [Circular *1],
npm error   status: null,
npm error   signal: null,
npm error   output: null,
npm error   pid: 0,
npm error   stdout: undefined,
npm error   stderr: undefined
npm error }
npm error
npm error Node.js v22.23.1
npm error A complete log of this run can be found in: /home/node/.npm/_logs/2026-07-28T01_05_16_278Z-debug-0.log

npm ci failed with exit code 1.

Qwen Code · sandboxed verification

@wenshao

wenshao commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Local verification report — real build + real-render E2E

Verified as maintainer in an isolated worktree at head 377d2f7 (base 9faa806), with node_modules cloned from a checkout on the identical lockfile. Verdict: the PR does exactly what it claims. Recommend merge. Four non-blocking observations below, one of which is worth a follow-up.

1. Reviewer test plan — all steps reproduced

Step Command Result
Focused tests vitest run client/App.test.tsx client/components/ChatPane.test.tsx 283 passed (2 files)
Full WebShell suite vitest run --config vitest.config.ts 2331 passed / 141 files
Package build npm run build --workspace @qwen-code/web-shell ✅ built; renderComposerFooter present in dist/index.js
Typecheck tsc -p tsconfig.json --noEmit ✅ 0 errors
Lint eslint packages/web-shell ✅ clean
Prettier (the 6 changed files) prettier --check ✅ clean

On the full-suite run: client/build-artifact.test.ts (10 tests) fails if you run the suite before npm run build — it reads ../dist/index.js. Run the build first and it is green. Not a PR issue, just an ordering prerequisite.

Author's formatting note confirmed accurate. GitModePopover.module.css and client/index.html fail prettier --check, but they are byte-identical to the base (sha 12caed4f / 61be3de2 on both sides) and are untouched by this PR. Pre-existing baseline debt.

Trial merge with current main (923e5ab) — the PR is 8 commits behind and App.tsx moved on main. Merge is conflict-free, the footer placement survives intact, and on the merged tree: 2340 passed / 143 files, focused tests 284 passed.

2. Mutation matrix — the new tests are not vacuous

I applied 17 block-anchored mutations to the two production call sites and re-ran the PR's own tests. 17/17 killed.

# Mutation App Pane
1 footer moved above the composer (order swap)
2 footer hoisted out of the composer container
3 disabled hardcoded false
4 isRunning hardcoded false
5 currentMode hardcoded
6 currentModel hardcoded
7 sessionName hardcoded undefined
8 footer wrapped in a <div>
9 footer render removed entirely

Every prop in the contract and both placements are genuinely pinned. Note #8: wrapping the footer in a wrapper element is itself a caught regression, so the "no wrapper element" shape is deliberate and locked.

⚠️ A methodology note for anyone repeating this: a naive str.replace("<28 spaces>disabled={isDisabled}", ...) silently hits the header block instead, because the header's 30-space indentation contains the 28-space anchor as a substring. My first pass reported two false survivors from exactly that. The table above is from block-anchored edits with uniqueness assertions.

3. Real-render verification (the tests all mock ChatEditor)

Both test files stub ChatEditor with a <div data-web-shell-composer>, so the ordering assertions are made against a mock. The real ChatEditor root does carry that attribute (ChatEditor.tsx:1863), so the marker is faithful — but I re-verified against the real component + real CSS via Playwright against a mock daemon, injecting a host renderComposerFooter into StandaloneApp.

Measured in the live DOM (not asserted from the mock):

MAIN     footerIsNextSiblingOfComposer=true  sameParent=true  composerBottom=854 < footerTop=868
CONTROL  footerFound=false  composer.nextElementSibling=null
SPLIT    pane[0] nextSibling=true sameParent=true  817 -> 831
         pane[1] nextSibling=true sameParent=true  817 -> 831

omitted vs provided

Split view, both panes, live render:

split view

4. Observations (none blocking)

(a) disabled means different things in different composer slots — worth a follow-up.

Three hooks share the identical WebShellComposerToolbarRenderInfo type, but are handed different values in the primary chat:

// App.tsx — what ChatEditor actually gets
disabled={isDisabled || isStartingNewSessionSuggestion || interactionBlocked || approvalOverlayActive}
// App.tsx — what the header AND this PR's footer get
disabled={isDisabled}   // only the first term

renderComposerToolbarStart/End/Right render inside ChatEditor and therefore see the full four-term state; renderComposerHeader and renderComposerFooter see only isDisabled. Measured live — typing ? opens the shortcuts dialog (dialogOpeninteractionBlocked) while the composer stays visible:

baseline      footer=false  header=false  toolbarRight=false
dialog open   footer=false  header=false  toolbarRight=true    <-- same type, different answer

disabled divergence

So a host writing "dim my footer content while the composer is disabled" gets it right in a split pane and wrong in the main chat. This is pre-existing behaviour inherited from renderComposerHeader, not a regression introduced here — and in ChatPane the footer's disabled={approvalActive} is exactly the value ChatEditor receives, so the split side is correctly aligned (that is what commit 4b1a645 fixed). Aligning the App-side header + footer with ChatEditor's full expression is a good separate PR; it should not hold this one up.

(b) The PR description's contract is stale. The body says the renderer "receives the same composer state contract used by the existing header hook (sessionId and inputDisabled)". The actual contract is WebShellComposerToolbarRenderInfo = { disabled, isRunning, currentMode, currentModel, sessionName? } — no sessionId, no inputDisabled. The code is right and does match the header hook; only the prose is wrong. Worth fixing in the body so hosts aren't misled.

(c) In split view the renderer is instantiated 3×, not 2×. App's primary chat view stays mounted-but-hidden behind the split, so it renders a third instance (measured: hiddenAncestor=_chatViewWrap_ _chatViewHidden_, rect 0×0, session=(none)). Only 2 are visible. This is a property of the chat view staying mounted and applies equally to the existing header hook — but it does sharpen the PR's own "the renderer runs for each composer instance" caveat: a host doing anything with side effects (analytics, portals, subscriptions) will fire it for an invisible pane too.

(d) No built-in spacing. The header hook is wrapped in <div className={styles.composerHeader}> with margin-bottom: 8px; the footer is rendered bare. Hosts must supply their own top margin — I had to add one in the harness above to get a visible gap. Intentional per the mutation matrix (#8), just worth documenting.

Recommendation

Merge. 33 production lines, no new abstractions, matching the existing hook pattern; tests are non-vacuous by measurement, and the real-render behaviour matches the mocked assertions in both the primary and split composers. (b) is a one-line description fix; (a) is a worthwhile follow-up that predates this PR.

中文说明

本地验证报告 —— 真实构建 + 真实渲染 E2E

以维护者身份,在隔离 worktree 中基于 head 377d2f7(base 9faa806)验证,node_modules 从 lockfile 完全一致的检出克隆而来。结论:本 PR 的行为与描述完全一致,建议合并。 下面列出 4 条不阻塞的观察,其中 1 条值得后续跟进。

1. Reviewer 测试计划 —— 全部复现

步骤 命令 结果
定向测试 vitest run client/App.test.tsx client/components/ChatPane.test.tsx 283 通过(2 个文件)
WebShell 全量测试 vitest run --config vitest.config.ts 2331 通过 / 141 个文件
包构建 npm run build --workspace @qwen-code/web-shell ✅ 构建通过;dist/index.js 中包含 renderComposerFooter
类型检查 tsc -p tsconfig.json --noEmit ✅ 0 错误
Lint eslint packages/web-shell ✅ 干净
Prettier(本 PR 修改的 6 个文件) prettier --check ✅ 干净

关于全量测试: 如果在 npm run build 之前跑全量测试,client/build-artifact.test.ts(10 个用例)会失败——它读取 ../dist/index.js。先构建再跑即全绿。这不是 PR 的问题,只是执行顺序的前置条件。

作者关于格式化的说明经核实准确。 GitModePopover.module.cssclient/index.html 确实无法通过 prettier --check,但它们与 base 逐字节相同(两侧 sha 均为 12caed4f / 61be3de2),且本 PR 未修改它们。属于既有的基线欠账。

与当前 main923e5ab)试合并 —— 本 PR 落后 8 个提交,且 App.tsxmain 上有变动。合并无冲突,footer 的位置完好保留;合并后的树上:2340 通过 / 143 个文件,定向测试 284 通过

2. 变异矩阵 —— 新增测试并非空转

我对两处生产调用点施加了 17 个按代码块锚定的变异,并重跑本 PR 自带的测试。17/17 全部被杀死。

# 变异 App Pane
1 footer 移到输入框上方(顺序对调)
2 footer 提到输入框容器之外
3 disabled 硬编码为 false
4 isRunning 硬编码为 false
5 currentMode 硬编码
6 currentModel 硬编码
7 sessionName 硬编码为 undefined
8 footer 外面套一层 <div>
9 完全移除 footer 渲染

契约中的每个 prop 以及两处位置都被真正钉住了。注意第 8 条:给 footer 套一层容器元素本身就会被测试捕获,说明「不加包裹元素」是刻意设计并且被锁定的。

⚠️ 复现时的方法论提醒:朴素的 str.replace("<28 个空格>disabled={isDisabled}", ...) 会悄悄命中 header 代码块,因为 header 那 30 个空格的缩进包含了 28 个空格的锚点作为子串。我第一轮就因此报出了 2 个假幸存者。上表来自带唯一性断言的按块锚定改写。

3. 真实渲染验证(PR 的测试全部 mock 了 ChatEditor

两个测试文件都把 ChatEditor 桩成了 <div data-web-shell-composer>,因此顺序断言是针对 mock 做出的。真实的 ChatEditor 根节点确实带这个属性(ChatEditor.tsx:1863),所以这个标记是忠实的——但我仍然用 Playwright 配合 mock daemon,把宿主的 renderComposerFooter 注入 StandaloneApp,针对真实组件 + 真实 CSS 重新验证了一遍。

在真实 DOM 中实测(而非从 mock 断言):

MAIN     footerIsNextSiblingOfComposer=true  sameParent=true  composerBottom=854 < footerTop=868
CONTROL  footerFound=false  composer.nextElementSibling=null
SPLIT    pane[0] nextSibling=true sameParent=true  817 -> 831
         pane[1] nextSibling=true sameParent=true  817 -> 831

(截图见英文部分:未传入 vs 传入对比、分屏双面板实时渲染。)

4. 观察(均不阻塞)

(a) disabled 在不同的输入框插槽里含义不同 —— 值得后续跟进。

三个钩子共享完全相同的 WebShellComposerToolbarRenderInfo 类型,但在主聊天视图中拿到的值并不一样:

// App.tsx —— ChatEditor 实际拿到的
disabled={isDisabled || isStartingNewSessionSuggestion || interactionBlocked || approvalOverlayActive}
// App.tsx —— header 以及本 PR 的 footer 拿到的
disabled={isDisabled}   // 只有第一项

renderComposerToolbarStart/End/Right 渲染在 ChatEditor 内部,因此能看到完整的四项状态;而 renderComposerHeaderrenderComposerFooter 只能看到 isDisabled。实测:输入 ? 打开快捷键对话框(dialogOpeninteractionBlocked),此时输入框仍然可见:

基线        footer=false  header=false  toolbarRight=false
对话框打开   footer=false  header=false  toolbarRight=true    <-- 同一类型,答案不同

也就是说,宿主若想实现「输入框禁用时把我的 footer 内容置灰」,在分屏面板中是对的,在主聊天视图中则是错的。这是从 renderComposerHeader 继承下来的既有行为,并非本 PR 引入的回归——而且在 ChatPane 中 footer 的 disabled={approvalActive}ChatEditor 收到的值完全一致,分屏这一侧是对齐的(这正是 4b1a645 这个提交修掉的问题)。把 App 侧的 header + footer 对齐到 ChatEditor 的完整表达式,适合作为独立 PR,不应阻塞本 PR。

(b) PR 描述中的契约已过时。 正文写「渲染器接收与现有 header 钩子一致的输入框状态契约(sessionIdinputDisabled)」。实际契约是 WebShellComposerToolbarRenderInfo = { disabled, isRunning, currentMode, currentModel, sessionName? }——既没有 sessionId,也没有 inputDisabled代码是对的,也确实与 header 钩子一致,只是这段文字写错了。建议修正正文,以免误导宿主接入方。

(c) 分屏视图中渲染器被实例化 3 次,而非 2 次。 App 的主聊天视图在分屏背后保持挂载但隐藏,因此会渲染出第三个实例(实测:hiddenAncestor=_chatViewWrap_ _chatViewHidden_,矩形 0×0session=(none))。可见的只有 2 个。这是聊天视图保持挂载所致,对现有的 header 钩子同样成立——但它也让 PR 自己提到的「每个输入框实例都会调用该渲染器」这条风险更具体了:宿主如果在渲染器里做了任何带副作用的事(埋点、portal、订阅),也会为一个不可见的面板触发一次。

(d) 没有内置间距。 header 钩子外面包了 <div className={styles.composerHeader}>,带 margin-bottom: 8px;footer 则是裸渲染。宿主需要自己提供上边距——我在上面的验证工装里就得手动加一个才能看到间隙。根据变异矩阵第 8 条这是刻意设计,只是值得写进文档。

建议

合并。 33 行生产代码,没有新抽象,完全遵循既有钩子模式;测试经实测并非空转,真实渲染行为与 mock 断言在主输入框和分屏输入框上都一致。(b) 只需改一行描述;(a) 是一个先于本 PR 就存在的、值得跟进的后续项。

@wenshao
wenshao added this pull request to the merge queue Jul 28, 2026
Merged via the queue into QwenLM:main with commit 15b8ad2 Jul 28, 2026
77 checks passed

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline.

中文说明

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

— qwen3.7-max via Qwen Code /review

Comment on lines +8596 to +8599
{CustomComposerFooter && (
<CustomComposerFooter
disabled={isDisabled}
isRunning={streamingState !== 'idle'}

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] CustomComposerFooter is rendered without a shell-owned wrapper div, while CustomComposerHeader (line 8459) is wrapped in <div className={styles.composerHeader}> which provides margin-bottom: 8px. This creates a spacing asymmetry — hosts providing both renderers see 8px above the composer (shell-provided) but 0px below it. — Concrete cost: every host consumer must independently discover and add margin-top to their footer renderer to achieve symmetric spacing.

Suggested change
{CustomComposerFooter && (
<CustomComposerFooter
disabled={isDisabled}
isRunning={streamingState !== 'idle'}
{CustomComposerFooter && (
<div className={styles.composerFooter}>
<CustomComposerFooter
disabled={isDisabled}
isRunning={streamingState !== 'idle'}
currentMode={currentMode}
currentModel={currentModel}
sessionName={sessionDisplayName}
/>
</div>
)}
中文说明

[Suggestion] CustomComposerFooter 渲染时没有 shell 提供的包裹 div,而 CustomComposerHeader(第 8459 行)被包裹在 <div className={styles.composerHeader}> 中(提供 margin-bottom: 8px)。这导致间距不对称——同时提供两个渲染器的宿主会看到输入框上方有 8px 间距(shell 提供),但下方为 0px。—— 具体成本:每个宿主消费者都需要自行发现并在 footer 渲染器中添加 margin-top 以实现对称间距。

— qwen3.7-max via Qwen Code /review

OrbitZore pushed a commit to OrbitZore/qwen-code that referenced this pull request Jul 28, 2026
…wenLM#7884)

Run 30319209722 posted `Sandboxed verification: fail` on PR QwenLM#7856 with
"The PR could not be built ... treated as a PR failure verdict rather
than an infrastructure failure." The PR changed six source files. The
install died here:

    Error: spawnSync .../web-templates/node_modules/esbuild/bin/esbuild
           ETXTBSY
        at validateBinaryVersion (esbuild/install.js:99:28)

ETXTBSY is npm writing a dependency's binary and that package's own
install script exec'ing it before the write is closed — a race, and one
the PR had no part in. The comment accused its author anyway.

Both sandbox lanes now retry `npm ci` once. `npm ci` removes
node_modules before installing, so the second attempt cannot inherit the
half-written file; the transient class is simply absorbed instead of
being classified.

Retrying rather than classifying is the point. The obvious fix — match
ETXTBSY in the log and downgrade to infra-error — would read text the
PR's own lifecycle scripts can print, which is exactly the forgeable
signal the verdict logic was rewritten to stop consulting: it let a PR
launder its own deterministic breakage into "infrastructure, please
re-run". A retry consults nothing. A genuinely broken tree fails twice
and still earns `fail`, and the second attempt is only ever paid on a
path that is already failing.

The build is deliberately not retried: a compile error is deterministic,
so a second run would only double the cost of an honest failure.

Because the install now gets two chances, the sentence that blames the
PR says so — "failed twice in a row". PREPARE_ATTEMPTS is initialised
rather than defaulted at the point of use, or an inherited value would
claim a single-shot `npm run build` had failed twice, which is the same
false accusation pointed the other way.

Mutation-verified 5/5: an unbounded retry, no retry at all, a retry on
the build, dropping the initialisation, and reverting the wording each
turn at least one test red. The retry itself is covered behaviourally —
the real step text runs against a stubbed `runuser`/`npm`, so the tests
count actual install attempts instead of asserting that a loop exists,
which would pass on both a loop that never retries and one that never
stops.

Co-authored-by: wenshao <wenshao@example.com>
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.1.

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

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants