fix(web-shell): realign two suites with shipped behavior to unbreak main CI (#11525) - #11530
Conversation
…ain CI (#11525) The brand PR #11244 landed on top of #11470 and left two web-shell suites failing on main, redding the Test job: - 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. Fixes #11525
Autofix E2E Report — #11525IssueMain-branch CI failed on ReproductionGitHub log access was unavailable from this environment (no credentials), so the failure was reproduced locally on the exact failing commit by running the test suites for the files that commit touched. Two independent breaks surfaced, both in
Pre-fix baseline of the full web-shell suite: 8 failed tests in 2 files (the two above), 7105 passed. Post-fix: 300 files / 7113 tests, all passed. FixTest-only changes; no production behavior was touched, because both production behaviors are deliberate and reviewed (the footer compaction from
Mutation evidence: each fix was verified in both directions — with the fix reverted the exact tests fail (the pre-fix baseline), with it applied they pass; the rest of the file's assertions (exact request sequences, StrictMode behavior, degradation breakpoints) are unchanged and still enforced. Verification
Not run: the integration-test job (the change is test-only and the failed CI job was the unit-test job) and macOS/Windows lanes (no local runners; covered by CI). 中文说明Autofix E2E 报告 — #11525问题main 分支 CI 在提交 复现本环境无法访问 GitHub 日志(无凭据),因此在确切的失败提交上本地复现:运行该提交改动的文件所属的测试套件。问题浮出水面,共两处相互独立的破坏,都在
修复前完整 web-shell 套件基线:2 个文件共 8 个测试失败(即上述两处),7105 个通过。修复后:300 个文件 / 7113 个测试,全部通过。 修复仅修改测试;未改动任何生产代码,因为两处生产行为都是经过评审的既定行为(
变异验证:每个修复都经过双向验证——回退修复时对应测试精确失败(即修复前基线),应用修复后通过;文件内其余断言(精确请求序列、StrictMode 行为、降级断点)均未改动、仍在生效。 验证
未运行:集成测试任务(本次改动仅涉及测试,且失败的 CI 任务是单元测试任务)以及 macOS/Windows 通道(本地无对应运行器,由 CI 覆盖)。 🧠 Handled by Qwen Code · model/模型 |
github-actions
Bot
added
the
review/self-reported
|
Thanks for the PR! Template looks good ✓ — every required heading is present, including the Risk & Scope bullets and a complete Chinese translation. Problem: observed, and I confirmed it independently rather than taking the description's word for it. Direction: aligned. Main has been red since #11244 landed, and this restores it without reopening either behavior the suites tripped over — the compact-footer degradation from #11470 and the brand fetch from #11244 both stay exactly as shipped. Repairing the test environment instead of the product is the right call: neither shipped behavior is the bug, and both were reviewed on their own PRs. Size: not applicable. No core paths are touched — both files sit under Approach: the scope feels right, and it is the minimal version of the fix. Two things worth saying out loud from reading it:
Risk: no elevated risk signals — the Stage 1e path scan matched nothing. One mechanical thing to flag early: the branch is 6 commits behind main, which is why Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需标题齐全，包含 Risk & Scope 的三个条目和完整的中文翻译。 问题： 属于已观测到的 bug，而且我没有只采信 PR 描述，是独立核实过的。main 在 方向： 对齐。自 #11244 合入后 main 一直飘红，本 PR 在不推翻这两个套件踩到的既定行为的前提下恢复绿色 —— #11470 的页脚紧凑化与 #11244 的品牌拉取都保持原样发布。修测试环境而不是改产品，这个判断是对的：两处已发布行为都不是 bug，且各自都在自己的 PR 上评审过。 规模： 不适用。未触及核心路径 —— 两个文件都在 方案： 范围合理，而且已经是最小改法。读代码后有两点值得明说：
风险： 无升级风险信号 —— Stage 1e 路径扫描没有命中。有一点机械性问题需要提前指出：本分支落后 main 6 个提交，这正是 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head 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 |
Code reviewWhat I would have done, before reading the diff. From the title and the "why" alone: two suites broke because #11470 and #11244 were each green on their own base and only conflict in combination. For the sidebar one, the assertion target stopped existing at the mount width — so either mount wide enough for the label to render, or rewrite the assertion against the compact-mode contract; mounting wide is correct, because that suite's subject is brand naming, and width degradation already has its own dedicated suite. For the loading one, the exact-request-sequence assertion has to learn about the new The choice I'd have watched for, and the PR got it right. The easy version of the loading fix is to loosen What I verified against the code (statically, at the PR's base
No correctness blockers, no AGENTS.md violations, no production code touched, no drive-by edits. Nothing to request changes over in the diff itself. CI test evidenceThis is an unattended CI run, so per the gate's rules I did not build, run, or check out anything from this branch — the evidence below is the PR's own CI, read through the API, plus the job logs from both arms. Both arms are now hard evidence, and neither rests on the description's say-so. Before — the red After — That is the 2 + 6 = 8 previously-failing tests passing, plus the other eight in the brand file still passing at the new width — which is the empirical answer to the one question static reading left open. One check is red, and it is worth being precise about what it is and isn't: That is Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Bot orchestration jobs ( No sandboxed lane is warranted here, and I want to say why rather than leave it looking overlooked: there is no product-behaviour claim in this PR to settle. It changes no production code, so 中文说明代码审查在读 diff 之前，我会怎么做。 只看标题和"为什么需要"：两套测试挂掉，是因为 #11470 与 #11244 各自在自己的基线上都是绿的，只在组合后冲突。侧边栏那套，断言目标在挂载宽度下已经不再存在 —— 那么要么以足够宽的宽度挂载让标签渲染出来，要么把断言改写成针对紧凑模式的契约；挂载更宽是对的，因为这个套件的对象是品牌命名，而宽度降级已经有专门的套件负责。加载那套，精确请求序列断言必须认识新的 我会重点盯的那个选择，PR 做对了。 加载套件最省事的修法是把 我对照代码核实过的内容（全部是静态核实，基于 PR 的基线
无正确性阻塞项，无 AGENTS.md 违规，未触及生产代码，无夹带改动。就 diff 本身而言，没有需要 request changes 的内容。 CI 测试证据这是无人值守的 CI 运行，因此按门槛规则，我没有对本分支做任何构建、运行或 checkout —— 下面的证据来自通过 API 读取的 PR 自身 CI，以及两侧的 job 日志。 现在两侧都有硬证据，都不依赖 PR 描述的自述。 修复前 —— main 在 修复后 —— 本 head 上的 即先前失败的 2 + 6 = 8 个测试全部通过，外加品牌文件里其余 8 个测试在新宽度下依然通过 —— 这正是静态阅读留下的唯一疑问的经验答案。 有一个检查是红的，值得精确说明它是什么、不是什么：（日志摘录见英文部分） 那是 （CI 表格见英文部分，由 finalize 工作流在 CI 收敛后原地更新。表中省略了机器人编排任务 —— 它们不是 PR CI。） 这里不需要沙箱通道，我想说明原因而不是让它看起来像被漏掉了：本 PR 没有需要落定的产品行为主张。它不改任何生产代码，所以 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the fix itself is clean and now empirically confirmed; the reservation is purely mechanical (the branch is 6 commits behind main, which is what turned the lint gate red), not doubt about the change. Stepping back: this is a small, honest PR that does one unglamorous thing correctly. I went in expecting the usual failure mode of a CI-repair PR — assertions quietly loosened until they can't fail — and that is exactly what it did not do. The strict The evidence ended up complete on both sides, which is rare for a test-only PR. Main's red job log at I also checked the thing that would have made this PR pointless or painful: main has moved 6 commits since the branch was cut, but none of them touched either test file, Why I'm not approving, even though the code is right and I'm deliberately not leaving an approve-on-green marker either. It would be pinned to For context, @yiliang114 approved One observation for the maintainer, not a defect in this diff and not something I'd block on: this is the second suite-level casualty of the same pattern — #11244 was green on its own base, #11470 was green on its own, and the breakage only existed in the combination, which no per-PR CI can see. Worth asking whether brand-related changes should have a post-merge main-CI check that catches this class earlier, since the autofix loop is currently what discovers it. Flagging it as a question, not proposing a rule. ⏸️ Deferring to @yiliang114 — the diff needs no changes, but it needs a rebase onto main to clear the lint-gate staleness check before it can be approved and merged. Since the author is the autofix bot, a human needs to decide who pushes that rebase. Needs a human call on this one. 中文说明Confidence: 3/5 —— 修复本身干净、且已被经验证据确认；保留意见纯属机械性（分支落后 main 6 个提交，这正是 lint 门槛变红的原因），而不是对改动本身的怀疑。 退一步看：这是一个小而诚实的 PR，把一件不起眼的事正确地做完了。我原本预期会看到 CI 修复类 PR 常见的失败模式 —— 断言被悄悄放宽到不可能失败 —— 而它恰恰没有这么做。请求序列上严格的 最终两侧证据都齐全了，这对一个纯测试 PR 来说并不常见。main 在 我也核查了那件会让本 PR 变得无意义或麻烦的事：自分支切出后 main 前进了 6 个提交，但没有一个提交改动这两个测试文件、 为什么我不批准，尽管代码是对的、 我也刻意没有留下 approve-on-green 标记。它会绑定在 作为背景：@yiliang114 在本次审查进行期间，于 UTC 06:25 批准了 给维护者的一点观察，不是本 diff 的缺陷，我也不会因此阻塞：这是同一模式造成的第二起套件级损失 —— #11244 在自己的基线上是绿的，#11470 也是绿的，破坏只存在于组合之中，而这是任何单 PR CI 都看不见的。值得问一句：品牌相关改动是否应该有一个合入后的 main-CI 检查，以便更早发现这一类问题，因为目前是靠 autofix 循环来发现的。这只是一个提问，不是提议规则。 ⏸️ 转交 @yiliang114 —— diff 无需修改，但需要 rebase 到 main 以清除 lint-gate 新鲜度检查，之后才能批准并合并。由于作者是 autofix 机器人，需要有人来决定由谁推送这次 rebase。这件事需要人来拍板。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 2": none — I finished the walk inside the tool budget (~13 calls)..
Not reviewed: reverse audit — stopped before round 1 by the review time budget.
Test Plan (not a blocker): client/components/sidebar/WebShellSidebar.brand.test.tsx — no such file or directory; client/components/WorkspaceSessionProvider.loading.test.tsx — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 2"：none — I finished the walk inside the tool budget (~13 calls).。
未审查：反向审计——评审时间预算不足，未能开始第 1 轮。
Test Plan（非阻断）：client/components/sidebar/WebShellSidebar.brand.test.tsx — no such file or directory; client/components/WorkspaceSessionProvider.loading.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.23.2)
| // Mount above the compact footer breakpoint (344px): below it the version | ||
| // label leaves the row (#11470), and the brand tooltip assertions query | ||
| // that label's title. | ||
| window.localStorage.setItem('qwen-code-web-shell-sidebar-width', '360'); |
There was a problem hiding this comment.
[Suggestion] The new beforeEach fixture hardcodes the sidebar-width storage key as a raw literal — a fifth copy of SIDEBAR_WIDTH_STORAGE_KEY, which WebShellSidebar.tsx:149 does not export — and the two tooltip assertions that depend on it carry no assertion message. Together they make the fixture's layout precondition invisible at the point of failure.
If SIDEBAR_FOOTER_COMPACT_WIDTH (currently 344, WebShellSidebar.tsx:154) is raised above 360, if the storage key is renamed, or if the sidebar stops reading its persisted width at mount, readSidebarWidth() falls back to SIDEBAR_DEFAULT_WIDTH = 260, the version label leaves the footer row, and both treats an empty name as unset rather than blanking the brand row and names the version tooltip after the brand fail with expected null not to be null. Nothing in that output mentions the sidebar width, the storage key or the footer breakpoint, so the natural read is a branding regression and the investigation goes into brandContext and the footer version badge instead of into this fixture.
Witness:
BASE (fixture intact, PR code as committed):
Test Files 1 passed (1) Tests 10 passed (10)
FLIP (writes of that one localStorage key suppressed):
x sidebar brand > treats an empty name as unset rather than blanking the brand row
AssertionError: expected null not to be null
at WebShellSidebar.brand.test.tsx:208:11
x sidebar brand > names the version tooltip after the brand
AssertionError: expected null not to be null
at WebShellSidebar.brand.test.tsx:216:11
Test Files 1 failed (1) Tests 2 failed | 8 passed (10)
The two failures are exactly the two this fixture feeds, and the printed output nowhere names the width, the key or the breakpoint. The sibling suite corroborates the mechanism unmodified: WebShellSidebar.footer-version.test.tsx mounts at 260 and asserts the version badge is null, then mounts at 360 and asserts it is non-null.
The fix spans three places, so it is written out rather than offered as a one-click suggestion: declare the key and the width as named locals at module scope (mirroring WebShellSidebar.footer-version.test.tsx:127 and its mountAtWidth helper), use the local in this setItem, and attach that sibling file's diagnostic-message idiom to the two tooltip queries.
const SIDEBAR_WIDTH_STORAGE_KEY = 'qwen-code-web-shell-sidebar-width';
const ABOVE_COMPACT_FOOTER_WIDTH = 360;
// in beforeEach:
window.localStorage.setItem(
SIDEBAR_WIDTH_STORAGE_KEY,
String(ABOVE_COMPACT_FOOTER_WIDTH),
);
// at each of the two tooltip assertions:
expect(
container.querySelector('[title="QiuQiu Code v1.2.3"]'),
'version tooltip missing — sidebar mounted below the compact footer breakpoint',
).not.toBeNull();Any width chosen here must stay above the breakpoint and inside the clamp: SIDEBAR_FOOTER_COMPACT_WIDTH = 344 (WebShellSidebar.tsx:154), clamped by Math.min(getSidebarMaxWidth(), Math.max(SIDEBAR_MIN_WIDTH, width)) (WebShellSidebar.tsx:593-594) with SIDEBAR_MIN_WIDTH = 220 / SIDEBAR_MAX_WIDTH = 420 (WebShellSidebar.tsx:151-152) — keep 360, because the 260 default does not clear 344.
中文说明
新增的 beforeEach fixture 把侧边栏宽度的 storage key 硬编码为裸字面量 —— 这是 SIDEBAR_WIDTH_STORAGE_KEY 的第五份拷贝,而 WebShellSidebar.tsx:149 并未导出该常量 —— 同时依赖它的两条 tooltip 断言都没有断言消息。两者合起来,使这个 fixture 的布局前提在失败点上完全不可见。
如果 SIDEBAR_FOOTER_COMPACT_WIDTH(当前为 344,WebShellSidebar.tsx:154)被提高到 360 以上、如果该 storage key 被重命名、或者侧边栏不再在挂载时读取持久化宽度,readSidebarWidth() 就会回退到 SIDEBAR_DEFAULT_WIDTH = 260,版本标签移出行,于是 treats an empty name as unset rather than blanking the brand row 与 names the version tooltip after the brand 两个用例都会以 expected null not to be null 失败。该输出中没有任何地方提到侧边栏宽度、storage key 或页脚断点,因此最自然的理解是品牌配置出现了回归,排查会走向 brandContext 和页脚版本徽章,而不是这个 fixture。
证据:
BASE(fixture 保持原样,即 PR 提交的代码):
Test Files 1 passed (1) Tests 10 passed (10)
FLIP(抑制该 localStorage key 的写入):
x sidebar brand > treats an empty name as unset rather than blanking the brand row
AssertionError: expected null not to be null
at WebShellSidebar.brand.test.tsx:208:11
x sidebar brand > names the version tooltip after the brand
AssertionError: expected null not to be null
at WebShellSidebar.brand.test.tsx:216:11
Test Files 1 failed (1) Tests 2 failed | 8 passed (10)
这两个失败正是该 fixture 所支撑的两个用例,而输出中完全没有出现宽度、key 或断点。姊妹测试套件在未改动的情况下印证了同一机制:WebShellSidebar.footer-version.test.tsx 在 260 下挂载并断言版本徽章为 null,在 360 下挂载并断言其非 null。
修复涉及三处,因此以代码块列出而不是一键 suggestion:在模块作用域把 key 与宽度声明为具名局部常量(参照 WebShellSidebar.footer-version.test.tsx:127 及其 mountAtWidth 辅助函数),在此 setItem 中使用该常量,并为两条 tooltip 查询加上该姊妹文件的诊断消息写法。
const SIDEBAR_WIDTH_STORAGE_KEY = 'qwen-code-web-shell-sidebar-width';
const ABOVE_COMPACT_FOOTER_WIDTH = 360;
// beforeEach 中:
window.localStorage.setItem(
SIDEBAR_WIDTH_STORAGE_KEY,
String(ABOVE_COMPACT_FOOTER_WIDTH),
);
// 两条 tooltip 断言处:
expect(
container.querySelector('[title="QiuQiu Code v1.2.3"]'),
'version tooltip missing — sidebar mounted below the compact footer breakpoint',
).not.toBeNull();此处选择的宽度必须保持在断点之上且落在 clamp 范围内:SIDEBAR_FOOTER_COMPACT_WIDTH = 344(WebShellSidebar.tsx:154),经 Math.min(getSidebarMaxWidth(), Math.max(SIDEBAR_MIN_WIDTH, width))(WebShellSidebar.tsx:593-594)裁剪,其中 SIDEBAR_MIN_WIDTH = 220 / SIDEBAR_MAX_WIDTH = 420(WebShellSidebar.tsx:151-152)—— 请保留 360,因为默认值 260 无法越过 344。
— qwen3.8-max via Qwen Code /review (v0.23.2)
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings.
Approval blockers: none.
Triage: Scan — test-only fix, no production code changes.
What I checked:
-
WorkspaceSessionProvider.loading.test.tsx—/brandmock returning 404 correctly simulates an older daemon without the brand route. The provider's brand fetch settles immediately without arming a retry timer, so the exact-call assertion is stable. The StrictMode double-issue (['GET /capabilities', 'GET /brand', 'GET /brand']vs['GET /capabilities', 'GET /brand']) correctly mirrors React StrictMode's remount behavior. -
WebShellSidebar.brand.test.tsx— persistingqwen-code-web-shell-sidebar-width: '360'before mount (matching the convention already used by the footer-version suite) ensures the sidebar mounts above the 344px compact footer breakpoint (#11470), so the version label and its tooltip actually render for the assertions. -
Cross-check: The PR description confirms the root cause is a semantic merge conflict between #11244 (brand) and #11470 (sidebar footer compaction) — each was green on its own base, the failures appear in combination. Before this PR: 8 tests failed across these 2 files; after: 7113/7113 pass.
No production behavior, API, or settings surface is touched.
Reviewed with AI assistance.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Re-submitting my approval with the reasoning attached — my earlier one on this head went out with an empty body, which records no evidence for anyone reading it later. No blocking finding.
For a test-only PR the whole review question is whether it aligns the tests with shipped behavior or weakens them to make red go green. Both halves here align, and I checked the two ways this could have been a quiet coverage loss.
The brand suite fix does not drop the compact-footer coverage
Pinning qwen-code-web-shell-sidebar-width to 360 in beforeEach makes every test in WebShellSidebar.brand.test.tsx mount above the 344px compact-footer breakpoint, so the obvious worry is that the below-breakpoint behaviour stops being tested anywhere. It does not: that behaviour is owned by a dedicated suite, WebShellSidebar.footer-version.test.tsx, which pins both sides of the breakpoint — drops the version label at the compact breakpoint along with the settings label mounts at 260px and asserts versionBadge(container) is null, keeps the version label hidden across the whole overlap range from #11453 walks the range, shows the settings label and the version label above the compact breakpoint mounts at 360px, and leaves the footer below the former tight breakpoint unchanged covers the fourth case.
So the division is right: the footer-version suite owns whether the label exists at a given width, and the brand suite owns what the label says. Before this change the brand suite was asserting a title on an element that #11470 deliberately removes at its own mount width — it was testing the wrong thing, not testing a behaviour that is now untested.
The loading-suite fix strengthens the assertion
This is the half I would have pushed back on if it had gone the other way. The exact-call assertion becomes ['GET /capabilities', 'GET /brand', 'GET /brand'] under StrictMode and ['GET /capabilities', 'GET /brand'] without — it now expects the brand fetch rather than excusing it.
Worth saying plainly because the alternative shape exists on another branch right now: #11524's 14a66491 repairs the same two files with calls.filter((call) => call !== 'GET /brand'), which strips the brand call out of the assertion and would keep passing if the brand fetch silently disappeared. This PR pins it instead. If both land, this is the version that should own the file.
The 404 is also the right choice and is documented at the call site: it models a daemon without the route, which settles the provider's brand fetch immediately instead of arming the bounded retry (BRAND_RETRY_DELAY_MS = 2_000, DaemonWorkspaceProvider.tsx:85), keeping the suite's timing deterministic rather than depending on a retry window. The brand-success path is covered elsewhere (App.test.tsx, the e2e mockDaemon), so answering 404 here does not leave the 200 branch untested.
The StrictMode asymmetry it records is real, not a test artifact
One capabilities call but two brand calls under a remount looks odd, so I traced it rather than accepting it. Capabilities is deduped through an in-flight promise cache — capabilitiesPromiseRef (DaemonWorkspaceProvider.tsx:112), reused at :150-159 when already set — so a StrictMode remount joins the existing request. The brand path has a deferred-disposal StrictMode guard (module sentinel at :76, cleanup comment at :229) but no equivalent in-flight dedupe, so the remount fetches again.
That is development-only: StrictMode double-invokes effects in development, so production issues one brand fetch. Not a defect and not worth changing here. Recording it only because the test now pins the asymmetry — if brand ever grows a retry storm or a real remount path starts double-fetching, giving it the same promise-ref dedupe would make the two symmetric and this expectation would need updating.
CI
The decisive check is in flight, not yet green: a fresh run has Test (ubuntu-latest, Node 22.x), Lint & Static and Capture web-shell visuals pending, with 10 checks already passing. That Test job is the acceptance criterion for this PR — it is the job that has been red on main since #11244 — so its result, not my reading, is what confirms the fix. My approval is on the code: the diff is exactly the two test files, no production file is touched, and both changes make the tests match behaviour that is already shipped and already pinned elsewhere.
中文说明
重新提交我的批准并附上推理——我先前在这个 head 上的那次批准正文是空的，对之后来读的人没有留下任何证据。没有阻塞项。
对纯测试 PR，评审的全部问题就是：它是让测试对齐已发布行为，还是为了让红变绿而弱化断言。这里两处都是对齐，而且我核查了两种可能造成静默覆盖丢失的情形。
brand 套件的修法没有丢掉紧凑页脚的覆盖
在 beforeEach 里把 qwen-code-web-shell-sidebar-width 钉成 360，会让 WebShellSidebar.brand.test.tsx 的每个测试都挂载在 344px 紧凑页脚断点之上，所以显而易见的担心是「断点以下的行为从此无人测试」。并没有：该行为由专门的套件 WebShellSidebar.footer-version.test.tsx 拥有，且断点两侧都钉住了——drops the version label at the compact breakpoint along with the settings label 在 260px 挂载并断言 versionBadge(container) 为 null，keeps the version label hidden across the whole overlap range from #11453 走遍该区间，shows the settings label and the version label above the compact breakpoint 在 360px 挂载，leaves the footer below the former tight breakpoint unchanged 覆盖第四个情形。
所以职责划分是对的：footer-version 套件拥有「某个宽度下标签是否存在」，brand 套件拥有「标签写的是什么」。本次改动之前，brand 套件是在一个 #11470 刻意在其挂载宽度下移除的元素上断言 title——它测的是错的东西，而不是测了一个现在无人测的行为。
loading 套件的修法强化了断言
这一半如果往另一个方向走，我是会打回的。精确调用断言现在在 StrictMode 下是 ['GET /capabilities', 'GET /brand', 'GET /brand']、非 StrictMode 下是 ['GET /capabilities', 'GET /brand']——它期望这次 brand 请求，而不是宽恕它。
之所以要说清楚，是因为另一种形态此刻就存在于别的分支上：#11524 的 14a66491 用 calls.filter((call) => call !== 'GET /brand') 修同样这两个文件，那会把 brand 调用从断言里剥掉，即使 brand 请求静默消失也照样通过。本 PR 改为钉住它。若两者都落地，该文件应由这一版拥有。
404 也是正确选择，且在调用点写了说明：它模拟没有该路由的 daemon，从而立刻让 provider 的 brand 请求落定，而不是启动那个有界重试（BRAND_RETRY_DELAY_MS = 2_000，DaemonWorkspaceProvider.tsx:85），让套件的时序保持确定、不依赖重试窗口。brand 成功路径在别处有覆盖（App.test.tsx、e2e 的 mockDaemon），所以这里回 404 不会让 200 分支失去测试。
它记录下来的 StrictMode 不对称是真实的，不是测试假象
一次 capabilities 调用但两次 brand 调用看着奇怪，所以我去追了而不是接受它。capabilities 通过一个在途 promise 缓存去重——capabilitiesPromiseRef（DaemonWorkspaceProvider.tsx:112），在已置位时于 :150-159 复用——所以 StrictMode 重挂载会加入既有请求。brand 路径有一个延迟销毁的 StrictMode 守卫（模块级哨兵在 :76，清理注释在 :229），但没有对应的在途去重，所以重挂载会再取一次。
这只发生在开发态：StrictMode 在开发环境下双调用 effect，因此生产环境只发一次 brand 请求。不是缺陷，也不值得在此改。之所以记录，是因为测试现在把这个不对称钉住了——如果 brand 将来出现重试风暴，或某个真实重挂载路径开始双取，给它同样的 promise-ref 去重会让两者对称，而这个期望值也需要随之更新。
CI
决定性的检查还在跑、尚未转绿：新的 run 里 Test (ubuntu-latest, Node 22.x)、Lint & Static 与 Capture web-shell visuals 处于 pending，另有 10 项已通过。那个 Test job 正是本 PR 的验收标准——它就是自 #11244 以来在 main 上一直红着的那个 job——所以确认修复的是它的结果，而不是我的阅读。我的批准是针对代码的：diff 恰好是这两个测试文件，没有触碰任何生产文件，两处改动都是让测试去匹配已经发布、且已在别处钉住的行为。
The Test lane on this branch was failing on WebShellSidebar.brand and the daemon route-surface guard, neither of which this branch touches — both were red on main, at and before the commit this branched from. #11530 realigned those suites with shipped behavior; merge it in so the lane reports on this change rather than on inherited breakage.
…ignment The same two web-shell test suites were realigned independently on main by #11530, so this branch's fix and the base branch's landed fix touched the identical lines. The executable code agreed exactly on both sides; only three comment lines differed in phrasing. Take main's already-landed wording, which also matches the neighbouring footer-version suite's "compact footer" term and the shipped SIDEBAR_FOOTER_COMPACT_WIDTH constant. The merge therefore has no net delta against main.
Pick up main's 2488d12 (fix(web-shell): realign two suites with shipped behavior to unbreak main CI, #11530). The Test job on this branch failed in packages/web-shell only -- WorkspaceSessionProvider.loading.test.tsx and WebShellSidebar.brand.test.tsx -- which this branch does not touch; the failures were main's own, already fixed there. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtva7llqva
Pick up main's 2488d12 (fix(web-shell): realign two suites with shipped behavior to unbreak main CI, #11530). This branch's Test job failed only in packages/web-shell (WorkspaceSessionProvider.loading.test.tsx and WebShellSidebar.brand.test.tsx), which it does not touch -- those were main's own failures, already fixed on main. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtva7llqva
…main shipped Commit 2488d12 from #11530 independently landed the same two web-shell test realignments this PR makes, so both sides edited the same logic rather than adjacent lines. WorkspaceSessionProvider.loading.test.tsx has a single fetch mock, and each side added a GET /brand 404 arm to it plus the identical strictMode-conditional calls assertion. The merge auto-combined both arms into the same if-chain, leaving the later endsWith-based arm unreachable behind the earlier exact-match arm from main. Keep the exact match: it cannot swallow an unrelated path into the brand reply, and it sits beside the other exact-match arm for /capabilities. WebShellSidebar.brand.test.tsx auto-merged without conflict. Main sets the 360px sidebar width once in beforeEach, which already covers the two tests this PR set it in, so those per-test writes survive as idempotent no-ops. Left as merged because the file did not conflict.
|
Released in v0.23.3. |
5 participants
What this PR does
This PR repairs the two
packages/web-shelltest suites that fail onmainat commit13b69b3313, which red theTest (ubuntu-latest, Node 22.x)CI job. Both failures are test-environment misalignments left behind by the brand-customization PR (#11244) when it landed on a main that already contained the sidebar footer compaction fix (#11470); no production behavior changes.The sidebar brand tests mount the sidebar at its default 260px width and assert the footer version tooltip's
title— but since #11470 the version label leaves the row below the 344px compact footer breakpoint, so the element is never rendered at that width. These tests now persist a 360px width before mounting (the convention the footer-version suite already uses), so the tooltip they assert on actually exists.The workspace-session loading suite pins the exact daemon requests issued on load. #11244 made the workspace provider fetch
GET /brandbeside capabilities, and this suite was not updated for it. Its fetch mock now answers/brandwith a 404 — an older daemon without the route, which settles the provider's brand fetch immediately instead of arming its retry timer — and the exact-call assertion now expects the brand fetch, including the double issue under StrictMode's remount.Why it's needed
Main CI has been red since #11244 merged. Each involved PR was green on its own base; the failures only appear in combination (a semantic merge conflict), plus one suite the brand PR's own updates missed. Fixing the two suites restores a green
Testjob on main without touching the deliberate, reviewed behaviors they tripped over.Reviewer Test Plan
How to verify
On this branch, run the two suites and the full package suite:
Expected: all pass. On
main(before this PR), the same commands fail with 8 tests across those 2 files: the brand suite cannot find[title="<Brand> v1.2.3"]because the label is hidden at the 260px default width, and the loading suite sees an unexpectedGET /brandin its exact request-sequence assertion. A reviewer can confirm the failure modes by checking outmainand running the same two files.Evidence (Before & After)
Before (full web-shell suite on
main):Test Files 2 failed | 298 passed (300),Tests 8 failed | 7105 passed (7113). After (this branch):Test Files 300 passed (300),Tests 7113 passed (7113).Tested on
Environment (optional)
N/A — unit tests only (vitest + jsdom).
Risk & Scope
Linked Issues
Fixes #11525
中文说明
本 PR 做了什么
本 PR 修复了在
main分支提交13b69b3313上失败的两套packages/web-shell测试,这两处失败导致Test (ubuntu-latest, Node 22.x)CI 任务变红。两处失败都是品牌定制 PR(#11244)合入一个已包含侧边栏页脚紧凑化修复(#11470)的 main 时留下的测试环境失配;不改动任何生产行为。侧边栏品牌测试以默认 260px 宽度挂载侧边栏,并断言页脚版本 tooltip 的
title—— 但自 #11470 起,版本标签在宽度低于 344px 紧凑页脚断点时会移出该行,因此在该宽度下元素根本不会渲染。这些测试现在在挂载前持久化 360px 宽度(沿用 footer-version 测试套件已有的惯例),使其断言的 tooltip 真实存在。workspace-session 加载测试套件精确断言加载时发出的 daemon 请求。#11244 让 workspace provider 在 capabilities 之外还会请求
GET /brand,而该套件没有相应更新。现在它的 fetch mock 对/brand返回 404 —— 等价于没有该路由的旧版 daemon,这会让 provider 的品牌拉取立即 settle,而不是启动重试定时器 —— 并且精确调用断言现在包含品牌请求(包括 StrictMode 重挂载下的两次发起)。为什么需要
自 #11244 合入后,main CI 一直飘红。相关 PR 各自在自己的基线上都是绿的,失败只在组合后出现(语义合并冲突),此外还有一套品牌 PR 自身更新时遗漏的测试。修复这两套测试可以在不触碰那些经过评审的既定行为的前提下,恢复 main 上
Test任务的绿色状态。评审者测试计划
如何验证
在本分支上运行这两套测试以及整个包的测试套件:
预期:全部通过。在
main(本 PR 之前)上,同样的命令会在上述 2 个文件中失败 8 个测试:品牌套件找不到[title="<Brand> v1.2.3"](因为标签在 260px 默认宽度下被隐藏),加载套件在其精确请求序列断言中遇到意外的GET /brand。评审者可以切到main运行同样两个文件来确认失败现象。前后对比证据
修复前(
main上的完整 web-shell 套件):Test Files 2 failed | 298 passed (300),Tests 8 failed | 7105 passed (7113)。修复后(本分支):Test Files 300 passed (300),Tests 7113 passed (7113)。测试平台
环境(可选)
N/A —— 仅单元测试(vitest + jsdom)。
风险与范围
关联 Issue
Fixes #11525