Skip to content

feat(web-shell): make the product name and logo configurable - #11244

Merged
wenshao merged 15 commits into
mainfrom
feat/web-shell-brand
Sep 10, 2026
Merged

wenshao merged 15 commits into
mainfrom
feat/web-shell-brand

Conversation

@wenshao

@wenshao wenshao commented Sep 7, 2026 •

Copy link
Copy Markdown
Collaborator

What this PR does

Makes the Web Shell's product name and logo configurable instead of hardcoded, so a deployment can white-label the shell without editing source and rebuilding. Two channels: an operator sets a name and a path to an SVG in settings.json, and the daemon resolves that into the branding the browser renders; an embedding host can instead pass a brand as a component prop, which replaces the resolved value outright. The name drives the sidebar brand row, the sidebar's version tooltip, the welcome header title, the About panel's version row label, and the browser tab title; the logo drives the sidebar mark and the favicon. The terminal banner already had its own white-label settings, so this follows their sanitization and path-resolution conventions rather than inventing parallel ones — one deployment's branding reads the same on both surfaces.

An installation that configures nothing renders exactly what it renders today. The resolved brand is empty, every consumer falls back to its own built-in literal, and the served document keeps its original title and favicon bytes, so the existing visual baselines still pass without re-capture.

Why it's needed

Renaming the shell meant editing six source files and rebuilding, and the logo existed as two hand-duplicated copies of the same inline SVG that had to be kept in sync by hand. Anyone deploying this behind their own brand had to carry a fork.

Two decisions carry the design, and both are about who controls what a browser renders. The workspace settings layer is never read — the route does not even load it from disk — because a workspace settings file usually arrives from a repository that the person opening the shell did not write, so it must not be able to rename the product or name a file for the daemon to read and inline into every connected browser. The same boundary covers environment placeholders: brand keys are read from each layer's pre-substitution snapshot, and a value that still contains $VAR/${VAR} is ignored with a warning, because placeholder substitution draws from the process-wide environment a workspace populates first at boot. And a custom logo is only ever rendered as an image, never injected as markup: SVG loaded through an image context cannot execute script, injected SVG can, and that invariant is what makes it safe for the daemon to skip sanitizing the artwork it read. Both properties are pinned by tests that fail if the invariant is broken.

The tab title and favicon are applied before first paint from a cached value, mirroring how the theme already avoids a flash, so a renamed deployment does not show the built-in name on every load. This needed no server-side rewriting of the served document: the static handler still sends the file untouched.

Reviewer Test Plan

How to verify

Build, then start a daemon with a brand configured and check both the wire contract and the rendered shell:

npm run build && npm run bundle
export QWEN_HOME=$(mktemp -d)
printf '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 8 8"><circle cx="4" cy="4" r="4" fill="#0a0"/></svg>' > "$QWEN_HOME/logo.svg"
cat > "$QWEN_HOME/settings.json" <<'JSON'
{ "security": { "folderTrust": false },
  "ui": { "brand": { "name": "QiuQiu Code", "logoPath": "logo.svg" } } }
JSON
node dist/cli.js serve --port 8731 --hostname 127.0.0.1

Then confirm:

  • curl -s localhost:8731/brand returns the name plus a data:image/svg+xml,… logo. Note the relative logoPath resolved against the directory of the settings file that declared it.
  • curl -s -H 'Accept: text/html' -o /dev/null -w '%{content_type}\n' localhost:8731/brand still returns application/json — the route sits ahead of the single-page-app fallback, so a browser-like Accept cannot coax the HTML shell out of it.
  • curl -s localhost:8731/capabilities advertises a web_shell_brand feature tag, so a host can preflight instead of relying on a 404 from an older daemon.
  • curl -s localhost:8731/ still serves the built-in <title>Qwen Code Web chat</title> and the original favicon bytes even though the brand is configured — the document is not rewritten server-side; the client applies branding and caches it for the next load's pre-paint script.
  • Open the shell in a browser: the sidebar, welcome header, About panel and tab title show the configured name, and the sidebar mark and favicon show the configured logo.
  • Now put a different brand in a workspace's own .qwen/settings.json inside the directory the daemon was started from, with folder trust disabled so that workspace is trusted. The response is unchanged: a repository cannot rebrand the shell. This is the security-relevant case.
  • Point the logo path at a missing file, a text file, a symlink, a directory, or an SVG over 32 KiB. Each time the route still answers 200 with the name and no logo, and the daemon's stderr explains why — a rejected logo degrades to the built-in mark rather than failing the shell.

For the embedded-host channel, pass a brand prop with a name and a logo node and confirm it replaces the daemon-resolved value, that the shell never writes the host page's title or favicon itself, and that an inline prop object plus an inline callback do not re-fire the resolution callback on every render.

Evidence (Before & After)

No screenshots were captured for this PR; the evidence below is from the wire and from the test suites.

Before (global qwen 0.23.0, feature absent): GET /brand → 404 for Accept: */* and application/json, but 200 with the HTML shell for Accept: text/html because the fallback claimed it; GET /capabilities had no brand field; a ui.brand block in settings.json produced no observable effect and no warning.

After (this branch, local build): GET /brand → {"name":"QiuQiu Code","logoDataUri":"data:image/svg+xml,%3Csvg%20xmlns%3D…"} for every Accept header; web_shell_brand advertised between workspace_skill_manage and workspace_settings; GET / still serving the built-in title with the favicon href's sha256 byte-identical to main; a trusted workspace's brand ignored in favour of the user layer; each rejection degrading to name-only with an operator-visible stderr line such as qwen serve: GET /brand: ui.brand.logoPath does not exist: /tmp/nope-missing.svg.

The default-path guarantee is checked three ways: the served index.html differs from main by the added pre-paint block plus two narrowing changes to the boot watchdog (its fatal-resource classification now excludes the favicon, which can never be load-bearing for boot, and its panel error list no longer records a content-free failure from the icon), with the favicon href's sha256 unchanged; a jsdom harness executes the real built document's inline scripts in parse order and confirms a first-ever load keeps the built-in title and logo while a cached brand applies both; and all 45 Playwright visual baselines pass without re-capture, which is the pixel-level proof that an unconfigured shell looks exactly as it did.

Test suites on the rebased branch: web-shell 282 files / 6400 tests, TypeScript SDK 38 files / 1820 tests, the CLI capability-registry cases, and the CLI brand/schema/settings cases all pass; build, bundle, tsc --noEmit for all three packages, Prettier and ESLint are clean. 50 Playwright smoke specs pass against the mocked daemon in a real browser.

One CI failure on the first run was this branch's own, and is fixed in a follow-up commit: the daemon developer index states how many capability tags the registry holds, a contract test enforces the agreement, and adding the new tag moved the registry to 158 while the index still said 157. That was the only failing test in CI's unit-test job. The same contract caught the count again after a later merge brought in an upstream tag; the index now reads 159. A separate integration gate timed out at its 20-minute step limit with zero test failures — 13 of 23 files complete, all passing, including this branch's own capability-ordering assertion — and the same timeout has hit unrelated branches, so it reflects that gate's budget rather than this change. A web-shell browser smoke job also failed to start its dev server with EMFILE: too many open files on the runner, which is environmental.

Locally the CLI package shows a set of red tests that do not reproduce in CI and are not from this change — most visibly a scheduled-tasks spec requesting a route that appears seven times in the test and zero times in the route source. Every affected file is byte-identical to main, and a base-tree measurement over the same selection found no net-new failing file or test name on this branch; they are load-sensitive timeouts on a busy machine.

Tested on

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

Environment

Local runtime: npm run build && npm run bundle, then node dist/cli.js serve on loopback with a temporary QWEN_HOME per case. Browser-side checks via the package's Playwright harness (smoke and visual configs) and a jsdom harness over the built document.

Risk & Scope

  • Main risk or tradeoff: the client renders a custom logo through an image context and never as injected markup, and the daemon deliberately does not sanitize the SVG it reads. That pairing is safe only as long as the invariant holds, so it is asserted in tests on both sides — a resolver test that a script-bearing SVG is accepted, and a render test that it reaches the document as an image src with no script node present. Anyone changing the sidebar to inline the logo must add a sanitizer first, and those tests are written to fail loudly if they do not.
  • Not validated / out of scope: Windows and Linux were not exercised locally, so the hardened read path (symlink, hard-link and FIFO refusals, O_NOFOLLOW) is only verified on macOS and by unit test elsewhere. The roughly forty localized strings that mention the product inside longer prose still say the built-in name — covering them needs a message-override mechanism the localization provider does not have, so a renamed shell still reads "Qwen Code" in body copy. The auth provider label still reads "Qwen OAuth" on purpose: that names the identity provider, not the product, and renaming it would misdescribe the flow to the person about to grant access. Terms-of-service, documentation and bug-report links still point at the real resources. The terminal CLI is unaffected and keeps its own separate banner settings.
  • Breaking changes / migration notes: none. The new settings default to empty, the new daemon field and route are additive, the new capability tag is additive, and the existing sidebar branding escape hatch keeps working and still takes precedence over the new prop. One localized string was removed — the About panel's version-row label, whose only purpose was to say the product name, which the brand name now supplies; it had no other consumer in either language table.

Linked Issues

None.

中文说明

这个 PR 做了什么

让 Web Shell 的产品名称与 Logo 变成可配置项而不再是硬编码,于是部署方可以做白标而无需改源码重新构建。两条通道:运维在 settings.json 里填名称和一个 SVG 路径,daemon 把它解析成浏览器要渲染的品牌;嵌入式宿主也可以改为通过组件 prop 传入品牌,此时整体取代解析出来的值。名称驱动侧边栏品牌行、侧边栏版本 tooltip、欢迎页标题、About 面板的版本行标签以及浏览器标签页标题;Logo 驱动侧边栏标记和 favicon。终端 banner 早已有自己的白标设置,因此这里沿用它的清洗与路径解析约定,而不是另造一套 —— 同一个部署在两个界面上的品牌表现一致。

未做任何配置的安装,渲染结果与今天完全相同。解析出的品牌为空,每个消费方回退到各自的内置字面量,服务端返回的文档保持原有的标题和 favicon 字节,因此现有视觉基线无需重录即通过。

为什么需要

改名过去意味着修改六个源文件并重新构建,而 Logo 以两份手工复制的相同内联 SVG 存在,必须靠人工保持同步。任何想把它放在自己品牌下部署的人都得长期维护一个 fork。

设计里有两条决定是承重的,都与「谁能控制浏览器渲染什么」有关。工作区设置层完全不被读取 —— 该路由甚至不会把它从磁盘加载进来 —— 因为工作区的设置文件通常来自一个打开 shell 的人并未撰写的仓库,所以它不能改名产品,也不能指定一个文件让 daemon 读取并内联进每一个连上的浏览器。同一条边界也覆盖环境变量占位符:品牌键读取的是各层替换前的快照,仍含 $VAR/${VAR} 的值会被忽略并给出告警,因为占位符替换取自进程级环境,而工作区在启动时优先填充它。而自定义 Logo 只作为图片渲染,绝不注入为 markup:通过图片上下文加载的 SVG 不能执行脚本,注入的可以,正是这条不变量让 daemon 可以不对读到的素材做净化。两条性质都由测试钉住,破坏不变量即失败。

标签页标题和 favicon 在首绘前由缓存值应用,沿用主题已有的避免闪烁机制,因此改名的部署不会每次加载都闪一下内置名称。这不需要服务端改写返回的文档:静态处理器仍然原样发送文件。

审阅测试计划

如何验证

构建后,用一个配置了品牌的 daemon 启动,同时检查线上契约与渲染结果:

npm run build && npm run bundle
export QWEN_HOME=$(mktemp -d)
printf '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 8 8"><circle cx="4" cy="4" r="4" fill="#0a0"/></svg>' > "$QWEN_HOME/logo.svg"
cat > "$QWEN_HOME/settings.json" <<'JSON'
{ "security": { "folderTrust": false },
  "ui": { "brand": { "name": "QiuQiu Code", "logoPath": "logo.svg" } } }
JSON
node dist/cli.js serve --port 8731 --hostname 127.0.0.1

然后确认:

  • curl -s localhost:8731/brand 返回名称加一个 data:image/svg+xml,… 的 Logo。注意相对的 logoPath 是按声明它的那个设置文件所在目录解析的。
  • curl -s -H 'Accept: text/html' -o /dev/null -w '%{content_type}\n' localhost:8731/brand 仍返回 application/json —— 该路由位于单页应用 fallback 之前,所以浏览器式的 Accept 骗不出 HTML 外壳。
  • curl -s localhost:8731/capabilities 广告了 web_shell_brand 特性标签,宿主可以先探测,而不必依赖旧 daemon 返回的 404。
  • 即使配置了品牌,curl -s localhost:8731/ 仍然返回内置的 <title>Qwen Code Web chat</title> 和原始 favicon 字节 —— 文档没有被服务端改写;品牌由客户端应用并缓存,供下次加载的首绘前脚本使用。
  • 在浏览器里打开 shell:侧边栏、欢迎页、About 面板和标签页标题显示配置的名称,侧边栏标记和 favicon 显示配置的 Logo。
  • 接着在 daemon 启动目录下的工作区自己的 .qwen/settings.json 里写一个不同的品牌,并关闭文件夹信任使该工作区被视为受信任。响应不变:仓库无法给 shell 改品牌。这是安全相关的一例。
  • 把 Logo 路径指向不存在的文件、文本文件、symlink、目录,或超过 32 KiB 的 SVG。每次都仍然返回 200、带名称、不带 Logo,且 daemon 的 stderr 说明原因 —— 被拒的 Logo 降级为内置标记,而不是让整个 shell 失败。

嵌入式宿主通道:传入带名称和 Logo 节点的 brand prop,确认它取代 daemon 解析值、shell 自身从不改写宿主页面的标题或 favicon,且内联的 prop 对象加内联回调不会在每次渲染时重放解析回调。

证据(前后对比)

本 PR 未采集截图;以下证据来自线上接口与测试套件。

改前(全局 qwen 0.23.0,功能不存在):GET /brand 对 Accept: */* 与 application/json 返回 404,但对 Accept: text/html 返回 200 加 HTML 外壳,因为 fallback 认领了它;GET /capabilities 没有品牌字段;settings.json 里的 ui.brand 块不产生任何可观察效果,也没有告警。

改后(本分支,本地构建):GET /brand 对每种 Accept 都返回 {"name":"QiuQiu Code","logoDataUri":"data:image/svg+xml,%3Csvg%20xmlns%3D…"};web_shell_brand 广告在 workspace_skill_manage 与 workspace_settings 之间;GET / 仍返回内置标题,favicon href 的 sha256 与 main 逐字节相同;受信任工作区的品牌被忽略、采用用户层的值;每种拒绝都降级为仅名称,并给出运维可见的 stderr 行,例如 qwen serve: GET /brand: ui.brand.logoPath does not exist: /tmp/nope-missing.svg。

默认路径的保证用三种方式核验:服务端返回的 index.html 相对 main 的差异是新增的首绘前代码块,加上对 boot watchdog 的两处收窄(其致命资源判定不再包含 favicon —— favicon 对启动永远不可能是 load-bearing —— 其面板错误列表也不再记录来自 icon 的无内容失败),favicon href 的 sha256 未变;一个 jsdom 装置按解析顺序执行真实构建产物里的内联脚本,确认首次加载保持内置标题与 Logo、而有缓存时两者都生效;45 个 Playwright 视觉基线全部通过且无需重录,这是未配置品牌的 shell 外观完全不变的像素级证明。

rebase 后分支上的测试套件:web-shell 282 文件 / 6400 用例、TypeScript SDK 38 文件 / 1820 用例、CLI 的 capability registry 用例、CLI 的 brand/schema/settings 用例全部通过;三个包的构建、打包、tsc --noEmit、Prettier 与 ESLint 均干净。50 个 Playwright smoke 用例在真实浏览器中针对 mock daemon 通过。

首次 CI 运行中有一个失败是本分支自己的,已在后续提交修掉:daemon 开发者索引里写死了 registry 中 capability tag 的数量,且有契约测试强制两者一致;新增标签使 registry 变成 158,而索引仍写 157。这是 CI 单测 job 里唯一失败的用例。之后一次合并又带入了一个上游标签,同一契约再次拦住计数,索引现为 159。另一个集成门禁在 20 分钟的步骤上限超时、零测试失败 —— 23 个文件跑完 13 个且全部通过,其中包含本分支自己的 capability 排序断言 —— 同样的超时也发生在无关分支上,因此那反映的是该门禁的预算,不是本改动。还有一个 web-shell 浏览器 smoke job 因 runner 上 EMFILE: too many open files 而没能启动 dev server,属环境问题。

在本地,CLI 包会显示一批在 CI 上不复现、也与本改动无关的红测试 —— 最明显的是一个 scheduled-tasks 用例请求的路由在测试里出现七次、在路由源码里出现零次。所有受影响文件与 main 逐字节相同,且针对同一选择的 base-tree 对照测量显示本分支没有新增的失败文件或失败用例名;它们是繁忙机器上对负载敏感的超时。

测试环境

OS 状态
🍏 macOS ✅
🪟 Windows ⚠️
🐧 Linux ⚠️

环境

本地运行时:npm run build && npm run bundle,然后每个用例用独立的临时 QWEN_HOME 在 loopback 上跑 node dist/cli.js serve。浏览器侧检查通过该包的 Playwright 装置(smoke 与 visual 两套配置)以及一个针对构建产物文档的 jsdom 装置。

风险与范围

  • 主要风险或取舍:客户端通过图片上下文渲染自定义 Logo、绝不注入为 markup,而 daemon 有意不净化它读到的 SVG。这个组合只在该不变量成立时才安全,所以两侧都有测试断言它 —— 解析器测试断言含脚本的 SVG 被接受,渲染测试断言它以图片 src 进入文档且文档中不存在 script 节点。谁要把侧边栏改成内联 Logo,就必须先加净化器,而这些测试被写成在不变量破坏时立即失败。
  • 未验证 / 不在范围内:Windows 与 Linux 未在本地跑过,因此加固读取路径(symlink、硬链接、FIFO 拒绝与 O_NOFOLLOW)只在 macOS 上实测,其余平台靠单测覆盖。约四十处正文里提到产品的本地化字符串仍然写内置名称 —— 覆盖它们需要本地化 provider 目前不具备的消息覆盖机制,所以改名后的 shell 在正文文案里仍会出现 “Qwen Code”。auth provider 标签仍然写 “Qwen OAuth”,这是有意的:那是身份提供方的名字而非产品名,改了会向即将授权的人错误描述该流程。服务条款、文档与缺陷上报链接仍指向真实资源。终端 CLI 不受影响,保留自己独立的 banner 设置。
  • 破坏性变更 / 迁移说明:无。新增设置默认为空,新的 daemon 字段与路由是增量的,新的 capability 标签是增量的,原有的侧边栏 branding 逃生口继续可用且优先级仍高于新 prop。有一条本地化字符串被删除 —— About 面板版本行的标签,它唯一的作用就是写出产品名,现在由品牌名提供;在两张语言表里都没有其他消费方。

关联 Issue

无。

White-labelling the Web Shell meant editing six source files and rebuilding: the
product name was a literal in the sidebar, the welcome header, the About panel
and the document title, and the logo existed as two hand-duplicated copies of the
same inline SVG.

Brand is now configuration. A name and a logo path are read from the operator
settings scopes, and a new process-global daemon route resolves the logo into a
data URI for the client. Embedding hosts get the same control through a prop that
replaces the resolved value outright. The terminal banner already had its own
white-label settings, so this follows their sanitization and path-resolution
conventions rather than inventing parallel ones.

Two decisions carry the design. The workspace settings layer is never read, so a
repository's own settings file cannot rename the product or name a file for the
daemon to inline into every connected browser. And a custom logo is only ever
rendered as an image, never injected as markup, which is what makes it safe for
the daemon to skip sanitizing the SVG it read.

An untouched installation renders exactly what it did before: the resolved brand
is empty, every consumer falls back to its own built-in literal, and the served
document keeps its original title and favicon bytes, so the existing visual
baselines stay valid.
@wenshao

wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

E2E test report

Full plan and results: .qwen/e2e-tests/2026-09-06-web-shell-brand-customization.md (local, git-ignored). Design rationale: docs/design/web-shell-brand-customization.md in this PR.

Baseline (pre-implementation, global qwen 0.23.0)

Recorded before any code existed, against the published CLI rather than a local build: GET /brand 404s for Accept: */* and application/json, but returns 200 with the HTML shell for Accept: text/html because the SPA fallback claims unmatched document navigations. GET /capabilities carries no brand field. GET / serves <title>Qwen Code Web chat</title> with the built-in favicon. A ui.brand block in settings.json produces no observable effect and no warning — nested unknown keys are never validated.

That Accept-header behaviour became a requirement on this PR: the new route had to be registered ahead of the fallback so it answers JSON for every Accept. Verified below.

Daemon contract (local build, real daemon, per-case QWEN_HOME)

Case Result
No brand configured {}, 200
Name + valid SVG {"name":"QiuQiu Code","logoDataUri":"data:image/svg+xml,…"}, decoding back to the source bytes
Relative logoPath resolves against the directory of the settings file that declared it — verified end to end
Accept: text/html / application/json / */* all application/json, 200 — registered ahead of the SPA fallback
Missing file / non-SVG / symlink / directory / oversize 200 with the name and no logo, each with an operator-visible stderr line, e.g. qwen serve: GET /brand: ui.brand.logoPath does not exist: …
Workspace-scope brand ignored. Run with security.folderTrust: false so the workspace is genuinely trusted — meaning a workspace value would be visible if the route read it. A workspace .qwen/settings.json naming Repo Controlled Brand was passed over in favour of the user layer's User Level Brand. The route also loads settings with the workspace layer skipped entirely, so it is never read from disk; asserted in the route test.
Layer precedence system over user over system-defaults, both directions, by unit test
Capability tag web_shell_brand advertised between workspace_skill_manage and workspace_settings

Document contract

The served index.html differs from main only by the added pre-paint block — 22 insertions, 2 deletions, both deletions being the boot-watchdog predicate this PR narrows. The favicon href's sha256 is byte-identical to main (1a14eb40…c2b3f), and the built-in <title> is unchanged, so the static handler was not turned into a body rewriter.

The pre-paint script is exercised two ways. Four unit cases run the real inline script against a fake document and storage: cached title and logo applied, title-only cache applied, no cache leaves both built-in values, corrupt cache leaves both built-in values and warns. Separately, a jsdom harness executes the real inline scripts from the built dist/web-shell/index.html in parse order and confirms a first-ever load keeps the built-in title and logo while a cached brand applies both. The ordering is load-bearing and pinned by test: the script must sit after the icon <link>, because it swaps that element's href. A counter-check reconstructs the pre-fix ordering and confirms the favicon swap does not happen there, so the pin is a real guard rather than a tautology.

Browser

  • 45 Playwright visual baselines pass without re-capture. The sidebar brand row and the welcome header are in those frames, so this is the pixel-level proof that an unconfigured shell is unchanged — the property that makes the change safe to ship.
  • 50 Playwright smoke specs pass in a real browser against the mocked daemon, which is what validates the mock's new route handling end to end (that file is excluded from the vitest suite, so nothing else covers it).
  • Custom logo rendering is asserted in jsdom: an SVG containing <script> reaches the document only as an img src, with zero script nodes present. The resolver's counterpart test asserts that same SVG is accepted — together they pin the invariant that makes the absence of a server-side sanitizer correct.

Suites

web-shell 282 files / 6400 tests · TypeScript SDK 38 files / 1820 tests · CLI brand, route, schema and settings-surface cases 121 · CLI capability-registry cases 11 — all pass on the rebased branch. Build, bundle, tsc --noEmit for all three packages, Prettier and ESLint clean.

The CLI package carries pre-existing red tests unrelated to this change: most visibly a scheduled-tasks spec requesting a /scheduled-tasks/legacy-run route that appears seven times in the test and zero times in the route source. Every affected file is byte-identical to main, and a base-tree measurement over the same selection — identical vitest invocation run against a worktree at the merge base — found no net-new failing file and no net-new failing test name on this branch.

Not covered

No screenshots; the evidence above is wire-level, DOM-level and pixel-baseline. Windows and Linux were not exercised locally, so the hardened read path (symlink, hard-link and FIFO refusal, O_NOFOLLOW) is verified on macOS and by unit test elsewhere. No Playwright spec drives a branded shell — the mock always answers an empty brand, deliberately, so the baselines stay valid; the branded path is covered by the jsdom and unit layers plus the manual daemon runs above.

The daemon index states how many capability tags the registry holds, and a
contract test fails the build when the two disagree. Adding the brand tag made
the registry 158 while the index still said 157.
@wenshao

wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

CI failure on the first run: mine, now fixed

Correcting the record, because my earlier read of this run was taken while jobs were still pending and reported only one of three failures.

1. Test (ubuntu-latest) — a real defect in this branch, fixed in 96fcfa6. The daemon developer index states how many capability tags the registry holds and a contract test enforces the agreement. Adding web_shell_brand moved the registry to 158 while the index still said 157, so capabilities-docs-contract.test.ts failed with expected 157 to be 158. With ANSI escapes stripped from the job log, that was the only failing test file in the whole unit-test job. I had added the capability tag late, during review fixes, and afterwards re-ran only the capability-registry cases and the brand files — not the directory containing this contract test. Verification-scope gap, not a code defect.

2. Integration Tests (no-AK, No Sandbox) — step timeout, zero test failures. has timed out after 20 minutes; 13 of 23 files complete, every one passing, including cli/qwen-serve-routes.test.ts (38 tests) which is where this branch's exact capability ordering is asserted against a real daemon. The slow files are unrelated SDK integration specs (689s, 374s, 269s). An unrelated branch hit the identical timeout the same morning, and 8 of 10 recent runs passed in 8m47s–20m14s — the gate sits close to its own ceiling.

3. web-shell E2E Smoke — environmental. EMFILE: too many open files on the runner; Playwright's dev server never started.

Also worth noting: the body's earlier paragraph about pre-existing red CLI tests described my local machine, not CI. Those files pass in CI; the body has been corrected.

wenshao and others added 13 commits September 7, 2026 14:14
The previous push arrived while a re-run of the prior commit was still holding
the workflow's concurrency slot, so GitHub suppressed the new run instead of
queueing it and the event does not replay once the slot frees. This leaves the
branch head with no CI at all. An empty commit re-fires the synchronize event;
the repository squash-merges, so it does not survive into main.
# Conflicts:
#	packages/web-shell/README.md
#	packages/web-shell/client/App.test.tsx
… and docs

- Expose a brandSettled flag from the workspace provider so a
  settled-with-no-brand outcome (older daemon, withdrawn host prop) is
  reported and invalidates stale cached chrome, while the in-flight
  state still fires nothing
- Fall back to the built-in sidebar mark when a logo data URI fails to
  decode, and when a host logo node is falsy; remount the logo image
  per data URI so a decode failure cannot stick to the next logo
- Report an empty host brand name as unset in onBrandResolved, and type
  the callback payload as the document-actionable subset
- Exclude the favicon from the boot watchdog's fatal-resource
  classification and keep its content-free error out of the panel list
- Require a namespaced <svg> root in the resolver, cover the post-decode
  size cap, and pin the exact stderr prefix the protocol doc publishes
- Assert GET /brand answers JSON on the real app for any Accept header
- Sync the capability index count (159), settings docs and the design
  doc with the above
… SVG scanner, test fixtures

- Settle the brand fetch only on a definitive outcome: an answer, or a
  404 from a route-less daemon. A retryable failure (503 bootstrap, 429,
  transport, old SDK) is unknown rather than absent — settling it
  cleared the pre-paint cache and reset the tab title mid-session over
  a blip nothing retries (review Critical)
- Rewrite the SVG root scanner quote-aware: a quoted `>`, `[` or `]` in
  DOCTYPE literals or the root tag no longer rejects valid documents,
  and an xmlns-shaped substring inside another attribute's value no
  longer certifies a namespace-less document as SVG
- Warn (not reject) when the root svg carries no viewBox and no
  width/height: the browser loads such an image successfully but may
  paint a blank mark at the sidebar's fixed size, and only the daemon's
  stderr can say so
- Treat a nullish host brand prop as absent in the resolution gate,
  matching the `??` one line above
- Pin previously unwitnessed guards: the fd identity re-check and both
  fs soft-fail branches, the logo key remount, the favicon error-list
  exclusion, the unsettled-to-settled transition, whole-object host
  takeover against a daemon logo, and the superseded-client reset and
  catch-leg disposed guards
- Drive GET /brand on a real app with the SPA fallback mounted and a
  token configured, and pin the System settings layer to empty files so
  a maintainer's machine-wide brand cannot leak into the assertions
- Round-trip the pre-paint cache through the real inline script instead
  of pinning each side against its own copy of the key
- Document the hard-link and decoded-size rules, the namespace
  requirement, the viewBox advisory, and the corrected 503 semantics
  (bootstrap catch-all, retryable; a draining daemon does not reject
  this route); the README's onBrandResolved row now names the narrower
  payload type
…n, scanner correctness, retry path

- Read brand keys from the pre-substitution snapshot and refuse
  environment placeholders with a warning: placeholder substitution
  draws from the process-wide environment, which a workspace populates
  first at boot, so resolving brand placeholders would let a repository
  supply the value — the workspace exclusion through a side door
  (review Critical)
- Blank quoted SVG attribute spans per UTF-16 code unit, not per code
  point: an astral character collapsed to one space and shifted every
  later index, refusing valid namespaced documents (review Critical)
- Accept prefix-bound <svg:svg> roots that bind xmlns:svg to the SVG
  namespace (browsers render them), keep refusing <svgfoo> near-misses
  and prefix-only bindings on an unprefixed root, and pin the
  whitespace-prefixed fake-xmlns shape that actually exercises the
  quote-blanking
- Make the scaling-geometry advisory value-aware: an empty viewBox,
  zero dimensions, or percentage width/height now warns (loads
  successfully but cannot scale into the sidebar box)
- Add refreshBrand beside refreshCapabilities — same 404-only settle
  rule, generation-guarded, gated to the genuinely-missing state and
  no in-flight fetch — and call it from the connection recovery path,
  so a retryable brand failure no longer leaves in-app chrome and tab
  chrome disagreeing for the page's lifetime
- Warn on console when the sidebar logo fails to decode (the one logo
  failure the daemon's root-tag validation cannot see)
- Pin the O_NONBLOCK FIFO defence with a real mkfifo fixture, the
  published 429/drain transport states on the real app, and make the
  ordering test's body discriminating with a fixture brand
- Document the placeholder carve-out, prefix-bound roots and the
  value-aware advisory across settings docs, schema, protocol reference
  and the design record
# Conflicts:
#	docs/developers/daemon/00-index.md
…retry machinery, scanner class fix

- Add QWEN_CODE_SYSTEM_SETTINGS_PATH / QWEN_CODE_SYSTEM_DEFAULTS_PATH to
  the hardcoded project-.env exclusions: they select which file becomes
  the System settings layer, so a repository could otherwise promote
  its own file above the operator's User layer and rebrand the shell
  with literal values (review Critical)
- Refuse brand placeholders on substitution, not syntax: run the real
  resolveEnvVarsInString and refuse only when it would change the
  value, so a literal name containing `$` survives while anything a
  workspace-tainted environment could have supplied is refused
- Emit one stderr line per brand warning instead of a joined line, so
  a log rule keyed on one key's prefix keeps firing when another
  misconfiguration exists
- Make the SVG root matcher prefix-agnostic and entity-aware: any
  prefix bound to the SVG namespace qualifies, character references in
  attribute values are decoded before comparison, a degenerate viewBox
  no longer suppresses the scaling advisory, and a prefix-bound root
  with unprefixed children gets its own advisory
- Retry a retryable brand failure once after a short delay and warn
  only when the retry is also refused (404 stays silent); wire
  refreshBrand into the recovery path with tests; gate it on
  in-flight; attribute retries per client instance; reset the brand in
  the render that observes a client change so no committed frame mixes
  two connections
- Pin the O_NONBLOCK flag with a flags-capture witness (the FIFO case
  alone cannot fail fast: a blocked open wedges the worker's event
  loop), skip the FIFO fixture on Windows where the defence does not
  exist, and mock the not-stattable path instead of relying on POSIX
  ENOTDIR
- Document the startup hold vs terminal 503 semantics correctly and
  retire the design record's pre-retry narrative
# Conflicts:
#	packages/web-shell/client/main.tsx
…s alignment, hold test

- Parse the logo with the vendored fast-xml-parser instead of the
  hand-rolled root-tag scanner: well-formedness (duplicate attributes,
  misplaced declarations) is now rejected by a real validator, the
  namespace check reads parsed attributes with entity references
  decoded, any prefix bound to the SVG namespace qualifies, and the
  prefix-children advisory walks the parsed tree — construct-aware by
  construction. The DOCTYPE is stripped for the validator with a
  quote- and subset-aware scan because the validator's own DOCTYPE
  handling is quote-blind (review Suggestion, filed as the class-level
  finding)
- Bound entity decoding at Unicode's ceiling: an out-of-range
  character reference no longer throws a RangeError out of the
  resolver — which the route's catch-all turned into a 200 {} that
  dropped the validly configured brand NAME along with the logo
  (review Critical)
- Refuse brand placeholders by comparing the layer's substituted value
  against its pre-substitution snapshot — the authoritative engine
  WITH the home-.env fallback — and keep an unresolvable placeholder
  verbatim so a typo'd variable shows as literal text; say so in all
  five published artifacts, and hint when a missing logo's path still
  contains a placeholder token (review Critical)
- Gate the prefix-children advisory on the default namespace being
  absent: with both bindings, unprefixed children render fine
- Split the scaling advisory's message by cause (zero-area viewport,
  malformed viewBox, no usable geometry) and document both advisories
- Pin the startup-hold guarantee with a real deferred-runtime test:
  GET /brand issued before the runtime is ready is held and answered
  200 once it settles
- Drive the two retry tests on fake timers (BRAND_RETRY_DELAY_MS is
  exported for them) instead of sleeping through the production
  constant, and pin the route's one-stderr-line-per-cause invariant
  with a two-misconfiguration case
Swap fast-xml-parser (subset validator plus pre-decoded attributes) for
saxes, the streaming XML parser jsdom uses: malformed documents are now
refused instead of shipped, attribute values are entity-decoded exactly
once so the xmlns gate cannot be faked by a second decode, and the parse
stream is closed after write so an unclosed root fails at EOF. Regenerate
both lockfiles for the dependency swap — the round-5 fast-xml-parser edge
was never recorded, which is what turned the frozen pnpm installs red —
and pin the refused well-formedness classes and the double-decode fixture
in the resolver tests.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
saxes was constructed with default options, so an undeclared namespace
prefix on a child (the classic legacy-export xlink:href with no
xmlns:xlink) parsed clean here while browsers fatal on it — the daemon
shipped a data URI no browser renders, with nothing on stderr. Enable
xmlns: true, matching jsdom (browsers always have namespace support),
flatten the object-shaped attribute values that mode returns, and pin
both sides: undeclared child prefixes refuse, implicitly-bound xml: and
properly declared bindings still accept. Mutation-checked: reverting the
option and the flattening turns the new refusal test red.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@wenshao

wenshao commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qqqys qqqys 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-only review at head 52fe16ef. No Critical is provable in what I read, and every recent blocking finding on this PR is verifiably fixed at this commit. This is a comment rather than an approval for one reason: the diff is ~6,400 lines across ~50 files and I did not read all of it in this run's budget, so the certification is incomplete — not because I found anything.

Previously blocking findings — verified fixed in the code at this head

Read packages/cli/src/services/web-shell-brand.ts as it stands at 52fe16ef, not the thread flags:

  • R4-2 (round 7): namespace well-formedness was not actually checked. parseSvgRoot now constructs new SaxesParser({ xmlns: true }) (:392), matching the option jsdom passes for XML documents, and flattens the object-shaped attribute values that mode returns (Object.fromEntries(… attr.value), :404-407). An undeclared prefix on a child is therefore an error rather than a silent accept, and the doc comment at :369-378 now states the namespace parity it has instead of overclaiming engine parity.
  • R4-2 (round 6): well-formedness was delegated to a subset validator. The hand-rolled matcher and fast-xml-parser's XMLValidator are gone; saxes enforces well-formedness over the whole document, and parser.write(content).close() (:421) is load-bearing — the comment records that without close() an unclosed root parses clean while a browser rejects it. The refusal is failed || rootLocal !== 'svg' || rootAttrs === undefined (:425-427), and a catch around the write sets failed, so a throw is a refusal, not a crash.
  • R6-2: attribute values were entity-decoded twice, which could complete a fake SVG namespace. decodeXmlEntities no longer exists anywhere in the file; saxes hands attribute values over decoded exactly once, and the namespace gate compares that single-decoded value — rootAttrs[binding] !== SVG_NAMESPACE (:432), with binding correctly chosen as xmlns or xmlns:<prefix> and a prefix binding on an unprefixed root explicitly not counting.
  • R6-1: a runtime dependency was added without regenerating either lockfile. The parser dependency is now saxes and both lockfiles are in this diff (package-lock.json, pnpm-lock.yaml) alongside packages/cli/package.json; Install (ubuntu|macos|windows-latest) are all green at this head, which is the lane that was red on the frozen-lockfile mismatch.

The surrounding controls in the same file are consistent with the threat model the code states: a 32 KiB cap checked both on stat and again on decoded bytes (:19, :230, :329), an 80-character name cap mirroring the TUI (:22, :167-171), path resolution through resolvePath plus the declaring settings file's directory (:180-190), FD identity re-verification after the lstat (:313), refusal of environment-placeholder values with an operator-visible warning (:135-143), and the explicit note that the client renders the logo as an img src data URI and must not be switched to inline markup without adding a sanitizer here first (:364-367).

Current scan

No blocking defect is provable in the resolver or its route. scalingIssueOf (:445-471) is advisory-only and cannot refuse a document, so its loose SVG grammar re-implementation cannot cost an operator a working logo. The round-8 review posted at this same commit returned zero findings under a Critical posting floor, its only Critical-class item being the deferred fails-closed one (DOCTYPE-declared entities are refused, so some design-tool exports are rejected) — a refusal with an operator-visible warning, not a wrong result. One thread remains open and it is Suggestion-level: no test drives the resolved brand through App to a rendered consumer.

CI at this head is green across the board — unit tests, lint and static checks, integration tests, Serve A/B, the web-shell visual capture, web-shell E2E Smoke, both Desktop Shell jobs and the live-host lane. Nothing is pending or failing.

What I did not read, and why this is not an approval

The web-shell client render path (App.tsx, brandContext.ts, WelcomeHeader.tsx, StatusMessage.tsx, WebShellSidebar.tsx, DaemonWorkspaceProvider.tsx, index.html, main.tsx, vite.config.ts), the SDK surface (DaemonClient.ts, daemon/types.ts, the export barrels), settingsSchema.ts and shared-env-keys.ts, serve/capabilities.ts and server.ts, the four documentation files, and the ~2,600 lines of tests. Approving means certifying the whole current diff, and I would be doing that on the strength of the resolver half alone.

Next step

No code change is requested — the blocking history is closed and the review at this head found nothing. Either a maintainer reads the client/SDK/settings half and lands it, or re-request this route and the remaining surface can be covered in a pass scoped to it.

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving. qqqys's Critical-only pass at this head verifies every recent blocking finding fixed in the code, not by thread flag: the SVG parser is now SaxesParser({ xmlns: true }) with write().close() load-bearing so unclosed roots and undeclared prefixes are refusals rather than silent accepts, and the double entity-decode is gone. The security invariants I care about hold: workspace settings are never read (a repo can't rebrand the shell), placeholder-bearing values are ignored pre-substitution, and the custom logo is only ever rendered through an image src (no script execution) with both sides pinned by tests.

CI is fully green (Lint, Test, web-shell E2E, Capture visuals, no-AK integration, Desktop Shell, Install). Same note as #11003: this is ~6,400 lines / ~50 files and neither qqqys nor I read every line in one pass — worth one maintainer end-to-end read before squash, not a block on this head.

@yiliang114
yiliang114 dismissed stale reviews from ghost September 10, 2026 04:04

Stale — filed against a prior head. Blocking findings are closed at 52fe16e (qqqys Critical-only verification + yiliang114 approval on the current head).

@wenshao
wenshao added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit 13b69b3 Sep 10, 2026
148 checks passed
qqqys pushed a commit to qqqys/qwen-code that referenced this pull request Sep 10, 2026
…ain CI (QwenLM#11525) (QwenLM#11530)

The brand PR QwenLM#11244 landed on top of QwenLM#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 QwenLM#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 QwenLM#11525

Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Shaojin Wen <szujobs@gmail.com>
Co-authored-by: 易良 <1204183885@qq.com>
yiliang114 added a commit that referenced this pull request Sep 10, 2026
Resolve the web-shell README props-table conflict: keep main's brand and
onBrandResolved rows (#11244) and append this PR's onAssistantTurnSettled
row, then let prettier re-pad the table.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-conflict/jmtva7llqva
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants