Skip to content

fix(web-shell): realign two suites with shipped behavior to unbreak main CI (#11534) - #11542

Open
qwen-code-dev-bot wants to merge 4 commits into
mainfrom
autofix/issue-11534
Open

fix(web-shell): realign two suites with shipped behavior to unbreak main CI (#11534)#11542
qwen-code-dev-bot wants to merge 4 commits into
mainfrom
autofix/issue-11534

Conversation

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

What this PR does

Realigns two web-shell test suites with the behavior that actually ships on main, unbreaking the Test (ubuntu-latest, Node 22.x) CI lane. The brand-configuration feature landed on top of the compact-footer change and the two suites were written against only one side of that combination: the sidebar brand tests mount at the default 260px width while asserting a footer version tooltip that the compact footer breakpoint (344px) now hides, and the workspace-session loading test pins the exact daemon requests issued on load without accounting for the brand fetch the provider now issues beside capabilities. The brand suite now mounts above the compact breakpoint (matching the footer-version suite's convention), and the loading suite answers the brand route with a 404 — an older daemon without the route — so the provider's brand fetch settles immediately instead of arming its retry timer, with the expected-call pin updated to include it. No production code changes; no assertion is weakened (the request pin becomes stricter).

Why it's needed

Main CI is red: the run on 53964ef7e2 failed in the Test job's unit-test step, and the failure reproduces deterministically on the current tree — 8 tests across the two suites. Every main push keeps failing this required check until the suites match shipped behavior.

Reviewer Test Plan

How to verify

Check out main without this PR and run the two suites in packages/web-shell: npx vitest run --config vitest.config.ts client/components/sidebar/WebShellSidebar.brand.test.tsx client/components/WorkspaceSessionProvider.loading.test.tsx. Expected: 8 failures — two tooltip-title assertions find no element (expected null not to be null), and six loading variants fail the exact-request pin with extra GET /brand calls. With this PR, the same command passes 16/16, and the full web-shell suite passes 300 files / 7113 tests. A reviewer should also confirm the assertions still test the real contract: the tooltip title still must read <brand name> v<version>, and the loading test still pins the exact request sequence (now including the brand fetch) rather than a superset.

Evidence (Before & After)

N/A (test-only change; no user-visible behavior). Before: 8 failed / 8 passed across the two suites. After: 16 / 16 passed, full package green.

Tested on

OS Status
🍏 macOS ⚠️ not tested
🪟 Windows ⚠️ not tested
🐧 Linux ✅ tested

Environment (optional)

Local Linux checkout, Node 22, npx vitest run in packages/web-shell plus repo-wide npm run build, npm run typecheck, npm run lint — all green.

Risk & Scope

  • Main risk or tradeoff: none user-facing; both edits align test fixtures with intentional shipped behavior (compact footer hides the version label below 344px; the provider fetches the brand beside capabilities). The 404 fixture mirrors exactly what an older daemon returns, which the provider treats as the definitive no-brand answer.
  • Not validated / out of scope: macOS and Windows lanes (Linux verified; the suites are DOM-level and platform-independent). A sibling autofix branch for the earlier same-signature issue Main CI failed: Qwen Code CI on 13b69b3313da #11525 carries the identical alignment; whichever lands second merges as a no-op.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #11534

中文说明

本 PR 做了什么

将两个 web-shell 测试套件与 main 上实际发布的行为重新对齐,修复 Test (ubuntu-latest, Node 22.x) CI 通道。品牌可配置功能合入时叠加在紧凑页脚改动之上,而这两个套件编写时只考虑了其中一侧:侧边栏品牌测试以默认 260px 宽度挂载,却断言一个会被紧凑页脚断点(344px)隐藏的页脚版本工具提示;工作区会话加载测试精确固定了加载时发出的守护进程请求,却没有计入提供方现在随 capabilities 一同发出的品牌请求。品牌套件现在改为在紧凑断点之上挂载(与 footer-version 套件的约定一致),加载套件则以 404 应答品牌路由——即不带该路由的旧版守护进程的响应——使提供方的品牌请求立即终结,而不再触发重试定时器,同时更新了预期调用断言以包含该请求。不改动任何生产代码;未削弱任何断言(请求固定断言反而更严格)。

为什么需要

main CI 当前为红:53964ef7e2 上的运行在 Test 任务的单元测试步骤失败,且该失败在当前代码树上可确定性复现——两个套件共 8 个测试失败。在套件与已发布行为对齐之前,每次 main 推送都会继续卡在这个必需检查上。

审查者测试计划

如何验证

在不包含本 PR 的 main 上检出代码,并在 packages/web-shell 中运行这两个套件:npx vitest run --config vitest.config.ts client/components/sidebar/WebShellSidebar.brand.test.tsx client/components/WorkspaceSessionProvider.loading.test.tsx。预期:8 个失败——两个工具提示标题断言查不到元素(expected null not to be null),六个加载变体因多出 GET /brand 调用而在精确请求断言上失败。应用本 PR 后,同一命令 16/16 通过,完整 web-shell 套件 300 个文件 / 7113 个测试全部通过。审查者还应确认断言仍在测试真实契约:工具提示标题仍必须是 <品牌名> v<版本>,加载测试仍固定精确的请求序列(现在包含品牌请求),而非一个超集。

前后对比(证据)

N/A(纯测试改动;无用户可见行为)。修复前:两个套件 8 失败 / 8 通过。修复后:16 / 16 通过,整个包为绿。

已测试平台

操作系统 状态
🍏 macOS ⚠️ 未测试
🪟 Windows ⚠️ 未测试
🐧 Linux ✅ 已测试

环境(可选)

本地 Linux 检出,Node 22,在 packages/web-shell 中运行 npx vitest run,以及仓库级 npm run buildnpm run typechecknpm run lint——全部通过。

风险与范围

  • 主要风险或取舍:无用户可见影响;两处修改均使测试夹具与有意为之的已发布行为对齐(紧凑页脚在 344px 以下隐藏版本标签;提供方随 capabilities 一同获取品牌)。404 夹具精确模拟旧版守护进程的真实响应,提供方将其视为确定性的无品牌应答。
  • 未验证 / 范围之外:macOS 与 Windows 通道(Linux 已验证;这些套件为 DOM 级测试,与平台无关)。针对更早的同一签名 issue Main CI failed: Qwen Code CI on 13b69b3313da #11525 的姊妹 autofix 分支带有完全相同的对齐修改;后合入的一方将以空变更合入。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

修复 #11534(Fixes #11534)

…ain CI (#11534)

The brand PR #11244 landed on top of #11470 and left two web-shell
suites failing on main, redding the Test job (run 34443429185):

- The sidebar brand tests assert the version tooltip's title while
  mounting at the default 260px width, but #11470 hides the version
  label below the 344px compact footer breakpoint, so the query finds
  nothing. Mount those brand tests at 360px, matching the convention of
  the footer-version suite.
- The workspace-session loading test pins the exact daemon requests
  issued on load, but the workspace provider now fetches GET /brand
  beside capabilities. Answer it with a 404 (an older daemon without
  the route, which settles the fetch instead of arming the retry timer)
  and expect the call, including the StrictMode remount double.

Both are test-environment alignments; no production behavior changes.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

Autofix Report — Issue #11534

Issue

A main-branch CI run on 53964ef7e2 failed in the Test (ubuntu-latest, Node 22.x) job at the Run tests and generate reports step (run 34443429185). The step runs the full workspace unit-test suite, so the fix had to be diagnosed from the code rather than from job logs (no log access in this environment).

Diagnosis

The failing commit itself only touched packages/qwen-live, but its ACP-adaptor tests pass locally, so I widened the search. The brand feature PR #11244 landed on main on top of the compact-footer PR #11470, and the combination left two web-shell suites red — reproduced deterministically on the current tree (8 failing tests):

  1. client/components/sidebar/WebShellSidebar.brand.test.tsx mounts the sidebar at the default 260px width. Since fix(web-shell): hide the sidebar version label at the compact footer breakpoint #11470, the footer version label — the element carrying the title="<Brand> v<version>" tooltip these tests query — is not rendered below the 344px compact footer breakpoint, so two assertions found nothing (expected null not to be null).
  2. client/components/WorkspaceSessionProvider.loading.test.tsx pins the exact daemon requests issued on load. Since feat(web-shell): make the product name and logo configurable #11244, DaemonWorkspaceProvider also fetches GET /brand beside GET /capabilities (verified in DaemonWorkspaceProvider.fetchBrand), so the pin failed on the extra call — twice under StrictMode, whose remount re-issues the fetch. Left unhandled, the mock's Unexpected request throw would additionally arm the provider's 2s brand-retry timer.

The rest of the web-shell suite (298 files / 7105 tests) was green before the change, confirming the scope.

Fix

Test-environment alignments only; no production behavior changes, and no assertion weakened:

  • The brand suite now mounts above the compact footer breakpoint (persisted width 360px, the same convention the footer-version suite uses), so the version label renders and the tooltip-title assertions query a real element again.
  • The loading suite answers GET /brand with a 404 — exactly what an older daemon without the route returns — which settles the provider's brand fetch immediately (the provider treats a 404 as the definitive "no brand here" answer) instead of arming its retry timer, and the expected-call pin now includes the brand request (doubled under StrictMode). The pin is stricter than before, not looser.

Mutation probe: for this test-only change the probe is inherent — the updated assertions were red on the pre-fix tree (8 failures reproduced above) and green after the edit, which is exactly the witness that they pin the new alignment. No production guard or branch was added, so there is nothing further to mutate.

Verification

  • npx vitest run --config vitest.config.ts client/components/sidebar/WebShellSidebar.brand.test.tsx client/components/WorkspaceSessionProvider.loading.test.tsx (in packages/web-shell) — before fix: 8 failed, 8 passed; after fix: 16 passed.
  • npx vitest run --config vitest.config.ts (full packages/web-shell suite) — 300 files / 7113 tests passed (before fix: 2 files / 8 tests failed).
  • npm run build — passed.
  • npm run typecheck — passed.
  • npm run lint — passed.
  • Integration tests — not run: the change touches only web-shell unit tests and is not exercised through the bundled CLI or the integration harness.
  • npm run generate:settings-schema — not applicable: no settings source changed.

Note: a sibling autofix branch for the earlier same-signature issue #11525 (origin/autofix/issue-11525) carries the identical alignment; whichever lands second merges as a no-op.

中文说明

Autofix 报告 — Issue #11534

问题

53964ef7e2 上的 main 分支 CI 运行在 Test (ubuntu-latest, Node 22.x) 任务的 Run tests and generate reports 步骤失败(运行 34443429185)。该步骤运行完整的工作区单元测试套件,因此本环境无法访问任务日志,只能从代码出发定位失败原因。

诊断

失败的提交本身只改动了 packages/qwen-live,但其 ACP 适配器测试在本地通过,因此我扩大了排查范围。品牌功能 PR #11244 在紧凑页脚 PR #11470 之后合入 main,两者叠加导致两个 web-shell 测试套件转红——在当前代码树上可确定性复现(8 个测试失败):

  1. client/components/sidebar/WebShellSidebar.brand.test.tsx 以默认 260px 宽度挂载侧边栏。自 fix(web-shell): hide the sidebar version label at the compact footer breakpoint #11470 起,页脚的版本标签——即这些测试查询的 title="<品牌> v<版本>" 工具提示所在元素——在低于 344px 紧凑页脚断点时不再渲染,导致两个断言查不到元素(expected null not to be null)。
  2. client/components/WorkspaceSessionProvider.loading.test.tsx 精确固定了加载时发出的守护进程请求。自 feat(web-shell): make the product name and logo configurable #11244 起,DaemonWorkspaceProviderGET /capabilities 之外还会请求 GET /brand(已在 DaemonWorkspaceProvider.fetchBrand 中核实),因此该固定断言因多出一次调用而失败——在 StrictMode 下则多出两次,因为其重新挂载会再次发起该请求。若不做处理,mock 抛出的 Unexpected request 还会额外触发提供方 2 秒的品牌重试定时器。

套件其余部分(298 个文件 / 7105 个测试)在修改前全部为绿,确认了影响范围。

修复

仅对齐测试环境;不改变任何生产行为,也未削弱任何断言:

  • 品牌套件改为在紧凑页脚断点之上挂载(持久化宽度 360px,与 footer-version 套件使用的约定一致),使版本标签得以渲染,工具提示标题断言重新查询到真实元素。
  • 加载套件以 404 应答 GET /brand——正是不带该路由的旧版守护进程的真实响应——这会立即终结提供方的品牌请求(提供方将 404 视为确定性的"此处无品牌"应答),而不会再触发重试定时器;预期调用列表的断言现在也包含该品牌请求(在 StrictMode 下为两次)。该断言比之前更严格,而非更宽松。

变异探针:对本次纯测试修改而言,探针是天然成立的——更新后的断言在修复前的代码树上为红(上方复现的 8 个失败),修改后转绿,这恰好证明它们固定了新的对齐行为。本次未新增任何生产守卫或分支,因此没有更多可变异的对象。

验证

  • npx vitest run --config vitest.config.ts client/components/sidebar/WebShellSidebar.brand.test.tsx client/components/WorkspaceSessionProvider.loading.test.tsx(在 packages/web-shell 内)——修复前:8 失败,8 通过;修复后:16 全部通过
  • npx vitest run --config vitest.config.ts(packages/web-shell 完整套件)——300 个文件 / 7113 个测试全部通过(修复前:2 个文件 / 8 个测试失败)。
  • npm run build —— 通过。
  • npm run typecheck —— 通过。
  • npm run lint —— 通过。
  • 集成测试 —— 未运行:本次改动仅涉及 web-shell 单元测试,不经过打包后的 CLI 或集成测试框架。
  • npm run generate:settings-schema —— 不适用:未改动任何设置来源。

注:针对更早的同一签名 issue #11525 的姊妹 autofix 分支(origin/autofix/issue-11525)带有完全相同的对齐修改;后合入的一方将以空变更合入。


🧠 Handled by Qwen Code · model/模型 kimi-k3 · CLI 0.23.2

@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Sep 10, 2026
@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required heading is filled in, and the bilingual body is a real translation rather than a stub.

Problem: observed, not theoretical, and I confirmed it independently rather than taking the description's word for it. I pulled the log for the failing job the linked issue names (run 34443429185, job Test (ubuntu-latest, Node 22.x), job id 102763782117) and it reports exactly Failed Tests 8, split the way this PR says:

  • components/sidebar/WebShellSidebar.brand.test.tsx (10 tests | 2 failed) — both expected null not to be null, at lines 204 and 212.
  • components/WorkspaceSessionProvider.loading.test.tsx (6 tests | 6 failed) — at line 199, AssertionError: expected [ 'GET /capabilities', 'GET /brand' ] to deeply equal [ 'GET /capabilities' ], and under StrictMode expected [ 'GET /capabilities', …(2) ] to deeply equal [ 'GET /capabilities' ].

That last line is the useful one: main really is issuing GET /brand at mount, so the request pin was stale, and the two arrays this PR now expects are precisely what the runtime already produces.

Direction: aligned. A required check is red on main and stays red on every push until the suites match shipped behavior. Nothing here touches a public contract, auth, sandbox, telemetry, or release surface — it is two test fixtures.

Size: 0 production lines, 18 test lines (17 additions / 1 deletion) across 2 files, 0 generated/schema. No core paths, so the core-module gate does not apply and there is nothing to escalate on size.

Approach: minimal, and I could not find a smaller correct version. One thing to settle before this merges, though:

Risk: no elevated risk signals. The revert-history path check matched nothing — both changed files are .test.tsx.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 所有必需小节都已填写,中英文正文是真正的对照翻译而非占位。

问题: 是已观测到的问题,不是理论性加固;而且我没有只采信 PR 描述,而是独立核实过。我拉取了关联 issue 所指失败任务的日志(run 34443429185,job Test (ubuntu-latest, Node 22.x),job id 102763782117),其中明确报告 Failed Tests 8,分布与本 PR 所述一致:

  • components/sidebar/WebShellSidebar.brand.test.tsx (10 tests | 2 failed) —— 两处均为 expected null not to be null,位于第 204、212 行。
  • components/WorkspaceSessionProvider.loading.test.tsx (6 tests | 6 failed) —— 位于第 199 行,AssertionError: expected [ 'GET /capabilities', 'GET /brand' ] to deeply equal [ 'GET /capabilities' ];StrictMode 下为 expected [ 'GET /capabilities', …(2) ] to deeply equal [ 'GET /capabilities' ]

最后一条尤为关键:main 上确实会在挂载时发出 GET /brand,说明请求固定断言已经过期,而本 PR 新填入的两个期望数组正是运行时当前的真实产出。

方向: 对齐。main 上有一个必需检查处于红灯,且在套件与已发布行为对齐之前,每次推送都会继续失败。本 PR 不涉及任何公开契约、鉴权、沙箱、遥测或发布面 —— 只是两个测试夹具。

规模: 生产代码 0 行,测试代码 18 行(新增 17 / 删除 1),涉及 2 个文件,生成/schema 0 行。未触及核心路径,因此核心模块门禁不适用,也无需就规模上报。

方案: 足够精简,我没能找到更小的正确版本。但合入前有一件事需要定夺:

风险: 无升级风险信号。回滚历史路径检查未命中 —— 两个变更文件都是 .test.tsx

进入代码审查 🔍

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

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

Screenshots · before / after

No screenshot changes against the PR base.

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

Qwen Code · web-shell visuals

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code review

No correctness blockers, and nothing that reads as an AGENTS.md violation. I went looking for the failure mode this shape of PR usually hides — a test "aligned with shipped behavior" that is really a regression being quietly accepted — and both edits check out against the production code. Details, because the reasoning is what makes this safe to merge:

The sidebar edit fixes a stale fixture, not a masked bug. SIDEBAR_DEFAULT_WIDTH is 260 and SIDEBAR_FOOTER_COMPACT_WIDTH is 344, so footerCompact (!collapsed && sidebarWidth < 344) is true at the default width, and the version span that carries title={${brandName} ${versionLabel}} lives inside the !collapsed && !footerCompact && versionLabel && … guard. querySelector('[title="QiuQiu Code v1.2.3"]') returning null is therefore correct shipped behavior from the compact-footer change, not a brand regression. Those were the only two assertions in the file reaching for that title, which is why exactly two tests failed.

Two things I specifically wanted to be sure of:

  • The clamp cannot silently undo the fix. getSidebarMaxWidth() is Math.max(420, floor(innerWidth * 0.5)), so clampSidebarWidth(360) is 360 regardless of jsdom's window width. That mattered: a max derived from innerWidth could otherwise have clamped 360 back under 344 and made the test environment-dependent.
  • 360 is an existing convention, not an invented number. The sibling WebShellSidebar.footer-version.test.tsx already mounts via mountAtWidth(360) using the same set-localStorage-then-mount mechanism, and the storage key matches SIDEBAR_WIDTH_STORAGE_KEY exactly.

Because the width goes into the shared beforeEach, it affects all 10 tests rather than the 2 that failed, so I read the other 8. They assert on brand-row text, the logo img versus the built-in inline mark, data-testid nodes, and console.warn — none of it footer-compactness dependent. The two negative text assertions (not.toContain('Qwen Code'), not.toContain('QiuQiu Code')) also still hold, because textContent does not include title attribute values: the newly rendered version span contributes only v1.2.3. No regression from the shared setup.

The 404 fixture is the correct one, not merely a convenient one. fetchBrand settles on exactly one outcome — error instanceof DaemonHttpError && error.status === 404setBrandSettled(true); return; — and every other failure arms setTimeout(…, BRAND_RETRY_DELAY_MS) with BRAND_RETRY_DELAY_MS = 2_000. This mock's fallback is throw new Error('Unexpected request: …'), a plain Error, so without the new branch the brand fetch would have taken the retryable path and armed a 2-second timer in all six variants. Answering 404 is what keeps the pin deterministic. It also models a genuinely supported deployment: DaemonClient.brand() documents "callers can preflight instead of relying on the 404 an older daemon returns; either way, treat branding as optional and fall back rather than fail", and failOnError is typed to return DaemonHttpError, so the status check really does match.

The StrictMode asymmetry is real, and the PR documents rather than papers over it. getCapabilities() memoizes its in-flight promise in capabilitiesPromiseRef and returns the same promise on re-entry, so the StrictMode remount never re-hits the wire. fetchBrand has no such memo — its cleanup bumps the generation and clears the timer but does not abort the request — so mount → unmount → mount yields two GET /brand against one GET /capabilities. The expected arrays match that precisely, including the …(2) shape the failing log showed.

Nothing else in the file needed touching. Every later calls assertion is calls.filter(call => call.endsWith('/events')), so the extra brand entries cannot disturb them; line 199 was the only exact-equality pin.

Two non-blocking notes:

  1. The StrictMode branch now couples this assertion to fetchBrand lacking in-flight dedup. If someone later gives it the memoization capabilities already has — a reasonable change, it would halve a dev-mode request — this array breaks and the failure will look cryptic. The comment the PR adds goes most of the way to covering that; just flagging it as the assertion most likely to need a revisit. This is dev-mode only, so it is not a user-facing defect today.
  2. Worth saying because it is not visible in the diff: unlike DaemonWorkspaceProvider.test.tsx, which mocks the SDK, this suite stubs only global fetch and drives the real DaemonClient. The new branch therefore exercises the genuine wire path end to end — real Response at status 404 → failOnErrorDaemonHttpError → the provider's 404-only settle rule. That is real coverage of the older-daemon contract, more than a fixture alignment.

The 18 added lines are all load-bearing for the stated goal: no drive-by refactors, no formatting churn, no unrelated edits. The comments are why-comments with issue references, which is the house style.

Test evidence

Unattended CI run, so per the gate's rules I did not build or execute anything from this PR — the evidence below is this repo's own CI read through the API, plus the base-branch failure log I pulled independently in Stage 1.

The base half of the claim is confirmed from real logs: job 102763782117 on run 34443429185 reports Failed Tests 8 across exactly these two suites, with the assertion output quoted in Stage 1. That is the reproduction, and it is GitHub's, not the author's.

The head half is not yet settled: the decisive lane is still running at the time of writing. Everything else that has finished is green, including Lint & Static and the web-shell visuals capture. I am reporting the pending lane as pending rather than guessing at it.

Final CI results for 4052e18 (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
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ 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,失败项排在最前。

Skipped by the workflow's own path filtering, so there is no signal to read either way: Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x), Integration Tests (CLI, No Sandbox), tmux-testing, verify. The PR body's "not tested" marks for macOS and Windows are consistent with that — those lanes did not run. Since both suites are jsdom DOM-level tests with no platform-specific surface, the Linux lane is the one that carries the claim.

Attribution, so this is not mistaken for evidence: the body reports 16/16 on the two suites and a full-package 300 files / 7113 tests green on the author's local Linux checkout. That is the author's own result and I did not re-run it. The pending Test lane above is what will confirm it.

No sandboxed lane needed here, and I want to be explicit about why rather than just omit it. The central claim is "these two suites fail on main and pass with this change" — which is precisely what Test (ubuntu-latest, Node 22.x) decides, and whose base half I already confirmed from the real failing job log. There is no user-visible behavior for @qwen-code /tmux to capture, and no runtime behavior for @qwen-code /verify to A/B against the base build: the diff touches only test files and the production surface is unchanged. Real-scenario terminal capture is N/A on this path (unattended CI never drives the product).

中文说明

代码审查

没有正确性阻塞项,也没有违反 AGENTS.md 的地方。我特意去找这类 PR 常见的隐藏问题——所谓"与已发布行为对齐"实际上是把一个回归悄悄接受下来——两处修改对照生产代码后都站得住。理由如下,因为正是这些推理让它可以放心合入:

侧边栏那处修改修的是过期夹具,不是被掩盖的 bug。 SIDEBAR_DEFAULT_WIDTH 为 260,SIDEBAR_FOOTER_COMPACT_WIDTH 为 344,因此默认宽度下 footerCompact!collapsed && sidebarWidth < 344)为真,而承载 title={${brandName} ${versionLabel}} 的版本 span 位于 !collapsed && !footerCompact && versionLabel && … 这个条件之内。所以 querySelector('[title="QiuQiu Code v1.2.3"]') 返回 null 是紧凑页脚改动带来的正确已发布行为,不是品牌功能的回归。文件里只有这两个断言会去取该 title,这也正是恰好失败两个测试的原因。

有两点我特别要确认:

  • 宽度钳制不会悄悄让修复失效。 getSidebarMaxWidth()Math.max(420, floor(innerWidth * 0.5)),因此无论 jsdom 的窗口宽度是多少,clampSidebarWidth(360) 都是 360。这一点很关键:由 innerWidth 推导出的上限本可能把 360 压回 344 以下,从而让测试依赖运行环境。
  • 360 是既有约定,不是临时编的数字。 同目录的 WebShellSidebar.footer-version.test.tsx 已经通过 mountAtWidth(360) 挂载,采用同样的"先写 localStorage 再挂载"机制,且存储键与 SIDEBAR_WIDTH_STORAGE_KEY 完全一致。

由于宽度写在共享的 beforeEach 里,它影响的是全部 10 个测试而不只是失败的 2 个,所以我把另外 8 个也读了。它们断言的是品牌行文本、logo img 与内置内联标记、data-testid 节点以及 console.warn——都不依赖页脚紧凑与否。两个否定式文本断言(not.toContain('Qwen Code')not.toContain('QiuQiu Code'))同样依然成立,因为 textContent 不包含 title 属性值:新渲染出的版本 span 只贡献 v1.2.3。共享 setup 不会带来回归。

404 夹具是正确选择,而不只是方便的选择。 fetchBrand 只在一种结果上落定——error instanceof DaemonHttpError && error.status === 404setBrandSettled(true); return;——其余任何失败都会以 BRAND_RETRY_DELAY_MS = 2_000 启动 setTimeout(…)。本 mock 的兜底是 throw new Error('Unexpected request: …'),一个普通 Error,因此若没有新分支,品牌请求会走可重试路径,并在全部六个变体中各自挂起一个 2 秒定时器。以 404 应答正是让固定断言保持确定性的原因。它也对应一个真实受支持的部署形态:DaemonClient.brand() 的文档写明"调用方可以预检,而不必依赖旧版守护进程返回的 404;无论哪种方式,都应把品牌视为可选并回退,而非失败",且 failOnError 的类型标注返回 DaemonHttpError,因此该状态码判断确实会命中。

StrictMode 下的不对称是真实存在的,而 PR 选择把它写清楚而非掩盖。 getCapabilities() 会把在途 promise 缓存在 capabilitiesPromiseRef 中,重入时返回同一个 promise,所以 StrictMode 重挂载不会再次触网。fetchBrand 没有这类缓存——它的清理只是递增 generation 并清除定时器,并不会中止请求——因此"挂载 → 卸载 → 挂载"会产生两次 GET /brand,对应一次 GET /capabilities。期望数组与此完全吻合,包括失败日志里显示的 …(2) 形态。

文件中没有别处需要改动。 后续所有 calls 断言都是 calls.filter(call => call.endsWith('/events')),多出的品牌请求不会干扰它们;第 199 行是唯一一处全等固定断言。

两条非阻塞提醒:

  1. StrictMode 那一分支现在把断言与 fetchBrand 缺少在途去重耦合在了一起。如果日后有人给它加上 capabilities 已有的那种缓存(这是合理的改动,能把开发态请求减半),该数组就会失败,且报错会显得莫名其妙。PR 新增的注释已在很大程度上覆盖了这一点;只是提示这是最可能需要回头调整的断言。这仅存在于开发态,因此目前不是面向用户的缺陷。
  2. 有一点从 diff 里看不出来,但值得说明:与 mock 掉 SDK 的 DaemonWorkspaceProvider.test.tsx 不同,本套件只打桩全局 fetch,驱动的是真实的 DaemonClient。因此新分支端到端地走了真实链路——状态码 404 的真实 ResponsefailOnErrorDaemonHttpError → 提供方的"仅 404 落定"规则。这是对旧版守护进程契约的真实覆盖,价值超出单纯的夹具对齐。

新增的 18 行全部服务于既定目标:没有顺手重构,没有格式化噪声,没有无关改动。注释都是带 issue 编号的"为什么"注释,符合本项目风格。

测试证据

本次为无人值守 CI 运行,按门禁规则我没有构建或执行本 PR 的任何代码——以下证据来自通过 API 读取的本仓库自身 CI,外加我在 Stage 1 独立拉取的基线分支失败日志。

主张的基线一侧已由真实日志确认:run 34443429185 的 job 102763782117 报告 Failed Tests 8,恰好分布在这两个套件,断言输出已在 Stage 1 引用。这就是复现,且出自 GitHub 而非作者。

head 一侧尚未定论:撰写时决定性通道仍在运行。其余已完成的全部为绿,包括 Lint & Static 与 web-shell 视觉快照。我把仍在运行的通道如实标为待定,而不是去猜结果。

上方表格中的 CI 状态由收尾工作流在 CI 结束后原地更新。

工作流自身路径过滤所跳过的检查(Test (macos-latest…)Test (windows-latest…)Integration Tests (CLI, No Sandbox)tmux-testingverify)没有任何可读信号。PR 正文中 macOS 与 Windows 标注"未测试"与此一致——那两个通道确实没有运行。由于两个套件都是 jsdom DOM 级测试、不涉及平台特有面,Linux 通道即为承载该主张的通道。

归属声明,以免被误当作证据: 正文称在其本地 Linux 检出上两个套件 16/16 通过、整包 300 文件 / 7113 测试全绿。这是作者自己的结果,我没有重跑。上方待定的 Test 通道才是确认它的依据。

此处不需要沙箱验证通道,我想明确说明理由而不是直接略过。 核心主张是"这两个套件在 main 上失败、应用本改动后通过"——而这恰恰由 Test (ubuntu-latest, Node 22.x) 判定,其基线一侧我已通过真实失败日志确认。没有用户可见行为可供 @qwen-code /tmux 采集,也没有运行时行为可供 @qwen-code /verify 与基线构建做 A/B:diff 只触及测试文件,生产面未变。真实场景终端采集在本路径下为 N/A(无人值守 CI 从不驱动产品)。

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — the code is correct as far as static review can establish and I could not break it; the two open items are a duplicate PR and an unfinished CI lane, neither of which is a defect in this diff.

My independent proposal, written before I read the diff, was the same two moves: mount the brand suite above the compact breakpoint, and teach the loading mock about the brand route while updating the request pin. The only place I would have differed is mechanical — I would probably have added a per-test width helper mirroring the sibling suite rather than putting the width in the shared beforeEach, to keep the blast radius at two tests. Having read all ten, that caution was unnecessary: nothing else in the file is width-sensitive, and the one-liner is simpler than my version. So the PR's approach is at least as good as mine, not a compromise of it.

The reason I am comfortable approving a bot-authored, test-only PR that makes failing tests pass — which is precisely the shape where an agent gets green by weakening an assertion — is that I checked for exactly that and it did not happen. The request pin gets stricter: an exact-equality array grows to include a request the runtime genuinely issues, and it stays an exact array rather than becoming a superset or a toContain. The tooltip assertions are unchanged in what they demand (<brand name> v<version> must still be the title); only the mount width moved, to a width where the element exists by design. Nothing was relaxed to reach green.

The numbers all line up against production source rather than against the PR's own narrative: 260 default versus the 344 breakpoint, a max-width clamp of max(420, …) that cannot pull 360 back under the breakpoint, the 404-only settle rule in fetchBrand against a 2000 ms retry timer that a plain Error would have armed, and the capabilities promise-memoization that explains why StrictMode doubles one request and not the other. That last one is the detail I would have expected a plausible-but-wrong fix to get backwards.

Six months from now this reads fine. The three comments cite the issues that explain the breakpoint and the brand fetch, so the next person does not have to reverse-engineer why the width is 360 or why the fixture answers 404. If anything the change leaves the suite better covered than it found it: this file drives the real DaemonClient rather than a mocked SDK, so the older-daemon 404 contract is now exercised end to end at the wire level, which the provider's own mocked unit tests do not do.

Two things I am not pretending are resolved:

  • fix(web-shell): realign two suites with shipped behavior to unbreak main CI (#11525) #11530 carries a byte-identical diff. I am approving the change, not adjudicating which of the two PRs should survive — that is a maintainer's call, and whichever merges second will need closing rather than merging. It is also a symptom worth a separate look: one root cause produced two issues and two PRs, and this author currently has eight autofix PRs open. I evaluated this one on its own evidence, but the duplication is the concrete cost of that volume and it will keep recurring until the pipeline dedupes by failure signature.
  • The decisive CI lane has not finished. Qwen Code CI is still in progress on the reviewed commit, and that lane is the only thing that can confirm the head half of the claim — that these two suites now pass. Everything else that ran is green, including lint and the web-shell visuals capture. I am not going to attest to a test result that does not exist yet, so approval is deferred until CI lands green on 4052e180e0de170384f7321de38ae0307e846cda; if it lands green the approval follows automatically against that exact commit, and if anything lands red it will be flagged instead.

Not a blocker, and I would not change it today: the StrictMode branch of the pin now asserts that the brand fetch is not deduplicated in flight. That is accurate dev-mode behavior, but it means a future improvement — giving fetchBrand the memoization capabilities already has — will fail this test with a confusing message. The added comment covers most of that; just the assertion most likely to need a revisit.

中文说明

信心度:4/5 —— 就静态审查所能确立的范围而言,代码是正确的,我没能找出破绽;两个未决事项是一个重复 PR 和一条尚未跑完的 CI 通道,二者都不是本 diff 的缺陷。

我在读 diff 之前独立形成的方案,与 PR 的做法是同两步:让品牌套件在紧凑断点之上挂载,并让加载 mock 认识品牌路由、同时更新请求固定断言。唯一会不同的是机械层面——我大概会照同目录套件的样子加一个按测试指定宽度的辅助函数,而不是把宽度写进共享的 beforeEach,以此把影响面控制在两个测试内。读完全部十个测试后,这份谨慎并无必要:文件里没有其他测试对宽度敏感,而那一行写法比我的版本更简洁。所以本 PR 的方案至少与我自己的相当,并非妥协产物。

我之所以能放心批准一个由 bot 提交、纯粹改测试、且作用是让失败测试通过的 PR——这正是 agent 最容易靠削弱断言换取绿灯的形态——是因为我专门查了这一点,而它没有发生。请求固定断言反而更严格:一个全等数组扩充到包含运行时确实会发出的请求,并且仍是全等数组,没有退化成超集或 toContain。工具提示断言所要求的内容没有变(title 仍必须是 <品牌名> v<版本>),变的只是挂载宽度,改到了一个按设计该元素本就存在的宽度。为了变绿,没有任何断言被放宽。

所有数字都与生产源码对得上,而不是与 PR 自己的叙述对得上:默认 260 对断点 344;最大宽度钳制为 max(420, …),不可能把 360 拉回断点以下;fetchBrand 中"仅 404 落定"的规则,对应一个普通 Error 本会触发的 2000 毫秒重试定时器;以及 capabilities 的 promise 缓存,解释了为何 StrictMode 只让其中一种请求翻倍。最后这一条,正是我预期一个"看着合理但实际错误"的修复会搞反的细节。

半年后再看也没问题。三条注释都引用了解释断点与品牌请求的 issue,因此后来者无需反推宽度为何是 360、夹具为何应答 404。如果说有什么变化,这套件被留下的覆盖比原来更好:本文件驱动的是真实的 DaemonClient 而非被 mock 的 SDK,因此"旧版守护进程 404"契约现在在链路层被端到端地执行了一遍,而提供方自己的 mock 单测做不到这点。

有两件事我不打算粉饰:

  • fix(web-shell): realign two suites with shipped behavior to unbreak main CI (#11525) #11530 携带逐字节相同的 diff。 我批准的是这个改动,而不是裁定两个 PR 中哪个该留下——那是维护者的决定,且后合入的那个应当被关闭而不是被合入。这也是一个值得单独关注的征兆:一个根因产出了两个 issue、两个 PR,而该作者目前有八个 autofix PR 处于开启状态。我是就本 PR 自身的证据做评估的,但这种重复正是该数量带来的具体代价,在流水线按失败签名去重之前会持续发生。
  • 决定性的 CI 通道尚未跑完。 在被审查的提交上,Qwen Code CI 仍在进行中,而它是唯一能确认主张另一半——即这两个套件现在通过——的通道。其余已运行的检查全部为绿,包括 lint 与 web-shell 视觉快照。我不打算为一个尚不存在的测试结果背书,因此批准推迟至 CI 在 4052e180e0de170384f7321de38ae0307e846cda 上变绿为止;若变绿,批准将自动针对该确切提交给出,若有任何检查变红,则会改为标记出来。

不构成阻塞,我今天也不会去改它:请求固定断言的 StrictMode 分支现在断言的是品牌请求在途被去重。这确实是开发态的真实行为,但意味着日后一项改进——给 fetchBrand 加上 capabilities 已有的那种缓存——会让该测试以令人困惑的报错失败。新增的注释已覆盖了大部分;只是这是最可能需要回头调整的断言。

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

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review timed out. Qwen review timed out after 5400 seconds (of the 90-minute budget). For large PRs, retry with a longer timeout by commenting: @qwen-code /review --timeout=360. See workflow logs.

@qwen-code-dev-bot

qwen-code-dev-bot commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

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

中文说明

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

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

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

Autofix review round — no action

No code change was made this round. There was no actionable feedback to
address.

Feedback triage

  • Reviews / inline comments: none.
  • [ic:5614982696] web-shell visual preview: informational — no screenshot
    changes against the PR base. Nothing to do.
  • [ic:5616156364] Qwen Code review timed out (check review-pr:
    FAILURE):
    the automated review workflow exceeded its 90-minute budget.
    This is CI infrastructure, not a defect in this PR: the diff is 17 lines
    across two test files, which cannot account for a 90-minute timeout. The
    suggested remedy (re-running the review with a longer timeout via a PR
    comment, or adjusting the review workflow's timeout) requires GitHub
    credentials this round does not have, and modifying .github/ CI machinery
    is outside what an autofix round is permitted to touch. Escalating to a
    maintainer rather than implementing anything here.
  • Still-red checks: none besides the timed-out review-pr above.

PR health spot-check

The two suites this PR touches were run locally and pass:

  • npm run build — passed
  • npx vitest run client/components/WorkspaceSessionProvider.loading.test.tsx client/components/sidebar/WebShellSidebar.brand.test.tsx (packages/web-shell)
    — 2 files, 16 tests, all passed
中文说明

Autofix 审查轮次 —— 无需改动

本轮没有进行任何代码改动,因为没有需要处理的可执行反馈。

反馈分诊

  • 审查意见 / 行内评论: 无。
  • [ic:5614982696] web-shell 视觉预览: 信息性通知 —— 与 PR 基线相比没有
    截图变化,无需处理。
  • [ic:5616156364] Qwen Code 审查超时(检查项 review-pr 失败): 自动
    审查工作流超出了 90 分钟的预算。这属于 CI 基础设施问题,而非本 PR 的缺陷:
    本 PR 的改动仅为两个测试文件共 17 行,不可能导致 90 分钟的超时。建议的补救
    措施(通过 PR 评论以更长超时重新运行审查,或调整审查工作流的超时配置)需要
    本轮不具备的 GitHub 凭据,且修改 .github/ CI 设施不在 autofix 轮次被允许
    触碰的范围内。因此升级为交由维护者处理,不在此处实施任何改动。
  • 持续失败的检查: 除上述超时的 review-pr 外无其他。

PR 健康状况抽查

本 PR 涉及的两个测试套件已在本地运行并通过:

  • npm run build —— 通过
  • npx vitest run client/components/WorkspaceSessionProvider.loading.test.tsx client/components/sidebar/WebShellSidebar.brand.test.tsx(packages/web-shell)
    —— 2 个文件、16 个测试,全部通过

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


🧠 Handled by Qwen Code · model/模型 kimi-k3 · CLI 0.23.2

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator Author

🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

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

Labels

review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Main CI failed: Qwen Code CI on 53964ef7e272

2 participants