Skip to content

feat(desktop-shell): restore the brand builder skill for the Tauri shell - #10165

Closed
yiliang114 wants to merge 1 commit into
mainfrom
feat/desktop-shell-brand-builder
Closed

feat(desktop-shell): restore the brand builder skill for the Tauri shell#10165
yiliang114 wants to merge 1 commit into
mainfrom
feat/desktop-shell-brand-builder

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

Restores the desktop brand builder skill for the Tauri shell at packages/desktop-shell/.agents/skills/desktop-brand-builder/, replacing the Electron-era skill that was removed together with packages/desktop in #9085. The skill keeps the original contract — a branded desktop package from a minimal brandId + logo — but targets the Tauri shell's branding hooks instead of the Electron build tree: src-tauri/tauri.conf.json (productName, identifier, shortDescription, updater endpoints), the src-tauri/icons/ set, and the bootstrap/ startup UI. The bundled brand-create.mjs is dependency-free plain Node (desktop-shell uses npm, not bun), regenerates the full icon set via tauri icon, and empties the updater endpoints by default so a branded build can never poll the official feed or be updated by it.

Why it's needed

The Electron removal in #9085 deleted the only white-label customization path (desktop-brand-builder + multi-brand support from #4581). The Tauri shell currently serves a single brand, and any downstream that wants a custom desktop client (the way OpenWork used the old skill before forking) has no entry point and would have to rediscover the branding hooks by hand. This restores that capability on the new architecture before the knowledge of what needs patching fades. Follow-up to the direction set in #8092.

Reviewer Test Plan

How to verify

cd packages/desktop-shell
cp -r . /tmp/ds-test   # scratch copy so the repo stays clean
sips -z 1024 1024 src-tauri/icons/icon.png --out /tmp/test-logo.png   # tauri icon needs >= 1024px
printf '{"brandId":"acme-ai","logo":"/tmp/test-logo.png","website":"https://acme.ai"}' > /tmp/brand.json
node .agents/skills/desktop-brand-builder/scripts/brand-create.mjs --shell-root /tmp/ds-test --config /tmp/brand.json

Confirm: summary JSON reports appName: "Acme AI", appId: "ai.acme.desktop"; /tmp/ds-test/src-tauri/tauri.conf.json has the new productName/identifier and empty updater endpoints; grep -c "Qwen Code" /tmp/ds-test/bootstrap/index.html /tmp/ds-test/bootstrap/bootstrap.js returns 0 for both; icons under /tmp/ds-test/src-tauri/icons/ are regenerated. Invalid input is rejected: a brandId of Bad_ID exits 1 with the regex message.

Evidence (Before & After)

N/A (agent skill + build script, no user-visible UI change). Local run output:

{
  "brandId": "acme-ai",
  "appName": "Acme AI",
  "appId": "ai.acme.desktop",
  "artifactPrefix": "Acme-AI",
  "updaterEndpoints": [],
  "icons": "regenerated via tauri icon",
  "bootstrapPatched": ["index.html", "bootstrap.js"]
}

Tested on

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

The Electron-era desktop-brand-builder skill was removed with packages/desktop in #9085, leaving the Tauri shell without a white-label customization path. Restore it against the new branding hooks: tauri.conf.json, the icon set, and the bootstrap UI. The bundled script is dependency-free Node, regenerates icons via tauri icon, and empties updater endpoints so branded builds never touch the official feed.
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR! Before the gate notes: this PR is a byte-for-byte duplicate of #10164 — same branch, same head SHA, same two files, created in the same second. Please close one of the pair so review doesn't run twice on the same commit.

  • Template: the three core sections and the full Reviewer Test Plan are there and well filled in. Missing the boilerplate bits: ## Risk & Scope, ## Linked Issues (the related PRs are cited in prose instead), and the Chinese <details> block. Not worth a gate stop on their own — please fold them in when you touch the branch next.
  • Problem: real and evidenced, not theoretical. feat(desktop): remove the Electron package after OpenWork fork; keep the Tauri upgrade bridge #9085 (merged yesterday) deleted packages/desktop together with the only white-label path — I confirmed .agents/skills/desktop-brand-builder/ was removed in that commit, and feat(desktop): add multi-brand support and build-for-distribution guide #4581 was the original multi-brand support. Build a lower-maintenance desktop app around Web Shell #8092 is the open desktop-direction issue this follows up on. The capability gap is documented.
  • Direction: restoring the white-label entry point on the Tauri shell, with a real downstream precedent (OpenWork used the old skill before forking). Doesn't touch auth/sandbox/telemetry/public contracts. Aligned.
  • Size: not applicable — no core-module paths; 2 new files under packages/desktop-shell/.agents/skills/.
  • Approach: scope feels right. Faithful port of the old contract (same brandId regex, same derived defaults, same minimal-input promise), dependency-free Node script, zero footprint on the shipping product — branding happens at build time in an isolated clone. No drive-by changes.
  • Risk: no high-risk path matches; no elevated risk signals.

Moving on to code review. 🔍

中文说明

感谢贡献!先说一件事:本 PR 与 #10164 完全重复——同分支、同 head SHA、同样两个文件、同一秒创建。请关闭其中一个,避免对同一个 commit 重复评审。

  • 模板:三个核心章节和完整的 Reviewer Test Plan 都在,且内容充实。缺少模板化的部分:## Risk & Scope## Linked Issues(相关 PR 已在正文中引用)以及中文 <details> 块。不至于卡门槛——下次改动分支时顺手补上即可。
  • 问题:真实且有证据,不是理论问题。feat(desktop): remove the Electron package after OpenWork fork; keep the Tauri upgrade bridge #9085(昨天合并)删除 packages/desktop 时也删掉了唯一的白标路径——我确认该 commit 移除了 .agents/skills/desktop-brand-builder/,而 feat(desktop): add multi-brand support and build-for-distribution guide #4581 正是最初的多品牌支持,Build a lower-maintenance desktop app around Web Shell #8092 是本 PR 跟进的桌面方向 issue。能力缺口有据可查。
  • 方向:在 Tauri shell 上恢复白标入口,有真实的下游先例(OpenWork 在 fork 前就用过旧技能)。不涉及 auth/沙箱/遥测/公共契约。方向一致。
  • 规模:不适用——未触及核心模块路径;仅在 packages/desktop-shell/.agents/skills/ 下新增 2 个文件。
  • 方案:范围合理。忠实移植旧契约(同样的 brandId 正则、同样的派生默认值、同样的最小输入承诺),脚本为无依赖的纯 Node,对出货产品零侵入——品牌化在隔离克隆中于构建时完成。没有夹带无关改动。
  • 风险:未命中高风险路径;无升级风险信号。

进入代码审查。🔍

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Code review

Reviewed against 43f0f700024bb7bd3f1d3da97081a8488bad571b. Before reading the diff, my own proposal for "restore white-label builds on the Tauri shell" was exactly what this PR does: a build-time patching skill living outside the shipped product (patch tauri.conf.json + icons + bootstrap in an isolated clone, then tauri build), not runtime branding support inside the shell. The PR matches that, and the contract is a faithful port of the removed Electron-era skill — I diffed it against the pre-#9085 version: same brandId regex, same derived defaults, same minimal-input promise. No duplication of live code, no drive-by changes.

Findings, most severe first:

1. CI is red on this PR's own code — lint fails. Test (ubuntu-latest, Node 22.x) fails with 7 ESLint no-undef errors in brand-create.mjs (process, console, URL). Root cause: the repo's node-globals override block in eslint.config.js covers packages/*/scripts/**/*.{js,mjs}, which does not match the new .agents/skills/.../scripts/ path, so the file is linted without Node globals. Natural fix: add packages/desktop-shell/.agents/skills/**/*.mjs to that block — it also carries no-console: 'off', which this script needs. This blocks merge.

packages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs
   25:17  error  'process' is not defined  no-undef
   26:23  error  'process' is not defined  no-undef
   30:3   error  'console' is not defined  no-undef
   31:3   error  'process' is not defined  no-undef
   62:24  error  'URL' is not defined      no-undef
   140:3  error  'console' is not defined  no-undef
   182:3  error  'console' is not defined  no-undef
✖ 7 problems (7 errors, 0 warnings)

2. The SKILL.md workflow can't actually finish a brand build as written. It installs only packages/desktop-shell deps in the clone, but npm run build:runtime (→ scripts/prepare-runtime.js) runs npm run build, bundle, and prepare:package at the repo root — which needs the root workspace's node_modules, never installed by the skill. On a fresh clone the packaging step fails. Since desktop-shell is deliberately excluded from the root workspace (!packages/desktop-shell), the fix is one extra line: npm install at the clone root before build:runtime.

3. Windows icon generation will silently degrade. generateIcons uses spawnSync('npx', ...) without a shell — on Windows npx is a .cmd shim and spawn fails with ENOENT, so the fallback path runs (only icon.png replaced; icon.ico for the NSIS installer stays Qwen-branded) with a warning. The author marks Windows ⚠️ untested, which is consistent. Consider shell: true (inputs are already validated) or resolving npx.cmd.

4. Known-limitation question, not a blocker: native surfaces keep the Qwen brand — src-tauri/src/main.rs (tray title, "Qwen Code update" dialogs, default workspace dir Qwen), runtime.rs, and the macOS Info.plist usage strings. The skill honestly scopes itself to conf + icons + bootstrap, but a white-label build will still show "Qwen Code" in the tray and updater dialogs. Worth a "known limitations" note in SKILL.md so downstreams aren't surprised, or a follow-up to patch those.

5. Minor: updaterEndpoints array items aren't validated as strings — a non-string item flows into tauri.conf.json. Cosmetic for a skill aimed at operators.

CI test evidence

From the PR's own CI on the reviewed commit, via the API — no PR code was executed in this run. The two duplicate PRs (#10164 and this one) share this head SHA, so two pull_request runs were triggered; one set of jobs was cancelled as duplicate. The failure below is in the active run and is caused by this PR (the errors are inside the file it adds). Post Coverage Comment failed downstream of it (no coverage artifact to post).

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

Check Conclusion
Post Coverage Comment (ubuntu-latest, 22.x) ❌ failure
Test (ubuntu-latest, Node 22.x) ❌ failure
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ 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,失败项排在最前。

Bot orchestration checks (triage, review-pr, verify, tmux-testing, …) are skipped or in flight and don't gate the PR.

What was not verified

The script's patching flow was not executed here — unattended runs never run PR code. The "Local run output" in the PR body is the author's claim (macOS only), not independent evidence; in particular the full build:runtimetauri build packaging path is untested by anyone so far (see finding 2). Sandboxed verification would settle both: @qwen-code /verify — it can run the documented test plan (brand-create on a scratch copy, assert the summary JSON, patched tauri.conf.json with empty updater endpoints, and zero remaining "Qwen Code" strings in bootstrap/) against a real checkout. The author has write access and can trigger it directly.

中文说明

代码审查(基于 43f0f700024bb7bd3f1d3da97081a8488bad571b):方案与我的独立设想一致——构建时在隔离克隆中打补丁,而不是在 shell 运行时里做品牌化支持;与 #9085 删除前的旧技能逐条对比,契约忠实还原,无重复代码、无夹带改动。

按严重程度列出的问题:

  1. CI 因本 PR 自身代码而红——lint 失败brand-create.mjs 有 7 个 no-undefprocess/console/URL)。原因是 eslint.config.js 的 node-globals 块只覆盖 packages/*/scripts/**/*.{js,mjs},不匹配新的 .agents/skills/.../scripts/ 路径。建议把 packages/desktop-shell/.agents/skills/**/*.mjs 加入该块(同时获得 no-console: 'off')。此项阻塞合并。
  2. SKILL.md 的流程按字面走无法完成打包:只在克隆里安装了 desktop-shell 依赖,但 build:runtime(prepare-runtime.js)会在仓库根执行 npm run build/bundle/prepare:package,需要根工作区的 node_modules。补一行:先在克隆根 npm install(desktop-shell 已被根工作区显式排除,两处都要装)。
  3. Windows 图标生成会静默降级spawnSync('npx', ...) 在 Windows 上因 .cmd shim 报 ENOENT,只会替换 icon.png,NSIS 用的 icon.ico 仍是 Qwen 品牌。作者也标注 Windows 未测试。建议 shell: true 或解析 npx.cmd
  4. 已知限制问题(非阻塞):原生界面仍保留 Qwen 品牌——main.rs 的托盘标题/更新对话框/默认工作区目录、runtime.rs、macOS Info.plist 用途说明。技能自身范围(conf+图标+bootstrap)是诚实的,但白标成品在这些地方仍会出现 "Qwen Code"。建议在 SKILL.md 加"已知限制"说明或作为后续跟进。
  5. 小问题updaterEndpoints 数组元素未校验为字符串。

CI 证据来自该 commit 的 API 数据,本运行未执行任何 PR 代码。两个重复 PR 共享同一 head SHA,触发了两次运行,其中一组作业被去重取消;上表失败属于活跃运行,且由本 PR 直接导致(错误就在其新增文件内)。

未验证项:脚本的补丁流程未在此执行;PR 正文中的本地输出是作者的自述(仅 macOS),不是独立证据,完整打包路径目前无人跑通(见问题 2)。可用 @qwen-code /verify 在沙箱中跑一遍文档中的测试计划来定案;作者有写权限,可直接触发。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 2/5 — right direction, minimal faithful design, but CI is red on the PR's own lint errors and the documented packaging flow can't complete as written.

Stepping back: this is a genuinely well-conceived PR. The instinct to restore the white-label entry point as a zero-footprint build-time skill — rather than adding branding hooks to the shipped Tauri shell — is exactly the minimal path, and the contract faithfully carries over the old skill's promise (two inputs in, branded package out). The empty-updater-endpoints default is a thoughtful safety detail. If I had to maintain this in six months, I'd thank the author.

But I can't approve what CI just rejected. The lint failure is squarely in the new file — the .agents/skills/** path slipped past the node-globals block in eslint.config.js — and it's a one-line config fix. The bigger substantive issue is finding 2 from my review: build:runtime builds the root workspace, which the skill's workflow never installs, so "one-click brand build" stops dead at the packaging step on a fresh clone. Both are quick fixes for a follow-up push; neither changes the design.

@yiliang114, two housekeeping items too: this PR is a duplicate of #10164 (same branch and SHA) — please close one of the pair; and the template's Risk & Scope / Linked Issues / Chinese sections are still missing, worth folding into the next push.

Requesting changes on the lint failure; the rest is in the review comment above. The direction itself has my full support. 🙏

中文说明

置信度:2/5 —— 方向正确、设计极简且忠实还原,但 CI 因本 PR 自身的 lint 错误而红,且文档中的打包流程按现状走不完。

退一步看:这是一个构思良好的 PR。把白标入口恢复为零侵入的构建时技能、而不是在出货的 Tauri shell 里加品牌化钩子——正是最小路径;契约忠实继承了旧技能的承诺(两个输入,产出品牌包)。默认清空 updater endpoints 是个细心的安全细节。半年后维护这段代码,只会感谢作者。

但 CI 刚刚拒绝了它,我不能批准。lint 失败就出在新增文件上——.agents/skills/** 路径没有落入 eslint.config.js 的 node-globals 块——一行配置即可修复。更实质的问题是审查中的问题 2:build:runtime 要构建根工作区,而技能流程从未安装根依赖,"一键品牌构建"在全新克隆里会卡死在打包步骤。两者都是下一次推送就能修掉的小改动,不影响设计。

@yiliang114,另有两件事务性事项:本 PR 与 #10164 重复(同分支同 SHA),请关闭其一;模板的 Risk & Scope / Linked Issues / 中文部分仍缺失,建议下次推送一并补上。

就 lint 失败请求修改;其余见上方审查评论。方向本身完全支持。🙏

Qwen Code · qwen3.8-max

Reviewed at 43f0f700024bb7bd3f1d3da97081a8488bad571b · 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.

CI lint is failing on the new file itself (7 no-undef errors), and the SKILL.md packaging steps can't complete without a root install — details in my review comment above. Both are quick fixes; the direction has my full support. 🙏

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Duplicate of #10164 — both PRs were created from the same branch in the same second due to a retried gh pr create call. Keeping #10164.

@yiliang114 yiliang114 closed this Aug 26, 2026
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.

2 participants