feat(desktop-shell): restore the brand builder skill for the Tauri shell - #10164
Conversation
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
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@yiliang114 The change itself reads as a faithful restore of the brand-builder skill on the Tauri shell (the branding hooks it targets in tauri.conf.json and bootstrap/ check out, and defaulting to empty updater endpoints is the right call) — but the PR body is missing required sections from the PR template, so this stops at the template gate:
## Risk & Scope— main risk or tradeoff, what is not validated, breaking changes## Linked Issues— the body references #9085 / #4581 / #8092 in prose; add the section with proper references<details>中文说明</details>— the template asks for a full Chinese translation of the English body
Once those are filled in, re-run triage (@qwen-code /triage) and it will continue to code review.
One more thing: #10165 is an exact duplicate of this PR (same title, same head SHA 43f0f700). One of the two should be closed.
中文说明
改动本身看起来是对 brand-builder skill 在 Tauri shell 上的忠实恢复（脚本针对的 tauri.conf.json 与 bootstrap/ 中的 branding 钩子均已核实存在，默认清空 updater endpoints 也是正确的做法）——但 PR 正文缺少 PR 模板 中的必填部分，因此在模板关卡停下：
## Risk & Scope—— 主要风险或权衡、未验证的内容、破坏性变更## Linked Issues—— 正文中以散文形式提到了 #9085 / #4581 / #8092，请补上该小节并正式关联<details>中文说明</details>—— 模板要求对英文正文做完整中文翻译
补齐后重新触发 triage（@qwen-code /triage）即可继续进入代码审查。
另外：#10165 与本 PR 完全重复（标题相同、head SHA 同为 43f0f700），两者应关闭其一。
— Qwen Code · qwen3.8-max
The desktop-brand-builder skill script runs under node but lives outside every existing node-scripts glob in eslint.config.js, so lint:ci failed with 7 no-undef errors on process/console/URL in brand-create.mjs. Add the skill scripts path to that existing block. Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
|
CI fixup for the
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src-tauri/icons/icon.png — no such file or directory; .agents/skills/desktop-brand-builder/scripts/brand-create.mjs — no such file or directory.
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
@qwen-code /triage |
1 similar comment
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 104 passed · 0 failed · 104 total Flakiness gate: not applicable — no runnable changed test files (0 out-of-scope file(s) noted in the log) 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：104 通过 · 0 失败 · 104 总计 抖动门：不适用 — no runnable changed test files (0 out-of-scope file(s) noted in the log) Verification reportPR #10164 verification — feat(desktop-shell): restore the brand builder skill for the Tauri shellVerdict: 中文摘要
Central claim + A/BCentral claim: running the bundled The base tree has no skill at all (the PR restores it), so the load-bearing A/B is split across the two deltas that actually have a control: the eslint glob (base config vs head config) and commit 3's updater hunks (head script vs the same script with exactly those hunks reverted). Witness captures beside each table. Cell table — happy path (head) —
35/35 assertions ( Cell table — commit-3 updater identity A/B —
15/15 ( Cell table — eslint glob A/B —
8/8 ( Reviewer Test Plan walkthrough
Corrections
FindingsF1 (Suggestion) —
Blast radius: any operator path with spaces (common on the two platforms this tool targets) silently ships a half-branded icon set; SKILL.md instructs the agent to author Minimal suggested fix (measured, not eyeballed) const result = spawnSync(
- 'npx',
+ process.platform === 'win32' ? 'npx.cmd' : 'npx',
['--yes', '@tauri-apps/cli', 'icon', brand.logo],
- { cwd: shellRoot, stdio: 'inherit', shell: true },
+ { cwd: shellRoot, stdio: 'inherit' },
);Measured in F2 (Suggestion) — custom F3 (Nit) — a directory passed as F4 (Nit) — overrides are unvalidated/unescaped. An F5 (Nit) — Not covered
MethodologyEnvironment: Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run stopped at the template gate again — no new review submitted, since the first pass's request-changes review still gates this PR. The PR body is still missing the same three required parts of the PR template:
Progress since the last pass is real: the /review responses read fine (R1-8/R1-17/R1-19 fixed in the latest commit, the rest declined with reasons), and the earlier sandboxed verification proved the branding flow end-to-end on the prior commit — with two non-blocking findings the code review will want to re-check against the newest commit (the 中文说明Triage 重跑再次停在模板关卡——首轮的 request-changes 评审仍然有效，因此不再重复提交新评审。 PR 正文仍然缺少 PR 模板 中的同样三个必填部分：
上一轮之后有实质进展：对 /review 发现的回复合理（R1-8/R1-17/R1-19 已在最新提交中修复，其余均给出了拒绝理由），早前的沙箱验证也在旧提交上端到端证明了 branding 流程——并留下两个非阻塞发现（ — Qwen Code · qwen3.8-max Reviewed at |
…awn on Windows - When branded build has no custom updater endpoints, also clear the official pubkey and set bundle.createUpdaterArtifacts to false so the Tauri bundler does not produce signed updater artifacts that no feed will serve. - Add shell: true to the npx spawnSync call so it works on Windows where npx is npx.cmd. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Add brand-builds/ to .gitignore (R1-19) - Validate logo is a file, not a directory (R1-8) - Document target-specific bundle path in SKILL.md (R1-17) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Review findings response (bdd9c7c)CriticalR1-4 (CI step installs only desktop-shell deps but build:runtime shells out at repo root): Declined — HYPOTHETICAL. R1-16 (build:runtime bundles for host platform-arch before cross-compile): Declined — HYPOTHETICAL. The SKILL.md shows FixedR1-8 (existsSync passes for directories): Fixed. Added R1-17 (Tauri bundle path mismatch for --target): Fixed. SKILL.md now documents that R1-19 (brand-builds/ not in .gitignore): Fixed. Added Declined (Suggestions)R1-5 (loop never verifies branding happened): The script reports what it did (which files were patched, icon result). Verification is the caller's responsibility per the SKILL.md Validation section. R1-6 (200-line script has zero tests): This is a skill automation script, not a library. Test infrastructure for the desktop-shell package is R1-7 (loadConfig assumes non-null object): R1-10 (website field has no read site): R1-15 (installerHooks untouched): The PR adds a brand builder skill; it does not modify the existing R1-9 (non-PNG logo warning claims icon.png was replaced): Minor cosmetic issue in the fallback path. The R1-14 (brandLogoName $ patterns in replacement): R1-18 (white-label promise not delivered on native OS surfaces): Native OS surface branding (About dialogs, system menus) is a feature enhancement, not a bug in the brand creation script. Out of scope for this PR. |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 96 passed · 0 failed · 96 total Flakiness gate: not applicable — no runnable changed test files (0 out-of-scope file(s) noted in the log) 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：96 通过 · 0 失败 · 96 总计 抖动门：不适用 — no runnable changed test files (0 out-of-scope file(s) noted in the log) Verification reportPR #10164 verification (round 2) — feat(desktop-shell): restore the brand builder skill for the Tauri shellVerdict: 中文摘要 — 判定：❌ 不通过 · 报告了发现（agent 判定）
Previous-finding status (follow-up round)All carried measurements were re-run at the new head
Delta of commit Central claim + A/BCentral claim: running the bundled The base tree has no skill at all (the PR restores it), so the load-bearing A/B again splits across the two deltas that have a control: the eslint glob (base vs head config) and commit 3's updater hunks (head script vs exact-hunk revert). Witness captures beside each table. Cell table — happy path + validation at head —
Cell table — commit-3 updater identity A/B —
The control cell is the red state commit 3 fixes; row 3 is the basis of F2. Cell table — eslint glob A/B —
Reviewer Test Plan walkthrough: every step re-executed at the new head — scratch copy + run (sips is macOS-only; substituted a sharp-generated 1024×1024 fixture), summary Corrections
FindingsF1 (Suggestion, stands) —
Blast radius unchanged from round 1: operator paths with spaces (common on both target platforms) silently ship a half-branded icon set — only Minimal suggested fix (re-measured at this head) const result = spawnSync(
- 'npx',
+ process.platform === 'win32' ? 'npx.cmd' : 'npx',
['--yes', '@tauri-apps/cli', 'icon', brand.logo],
- { cwd: shellRoot, stdio: 'inherit', shell: true },
+ { cwd: shellRoot, stdio: 'inherit' },
);Measured in F2 (Suggestion, stands) — custom F6 (Nit, new at this head) — non-string - const brandId = input.brandId?.trim();
- const logo = input.logo ? resolve(input.logo) : undefined;
+ const brandId = typeof input.brandId === 'string' ? input.brandId.trim() : undefined;
+ const logo = typeof input.logo === 'string' && input.logo ? resolve(input.logo) : undefined;Measured: all three type cells → exit 1 with the clean F3 residual (Nit, stands) — fallback warning is inaccurate for failing non-png logos. Measured: a corrupt F4 (Nit, stands) — overrides are unvalidated/unescaped. Re-measured: F5 (Nit, stands, widened) — Not covered
MethodologyEnvironment: Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run stopped at the template gate again — no new review submitted, since the existing request-changes review from the first pass still gates this PR (its @yiliang114 the PR body is still missing the same three required parts of the PR template:
The findings response and the CI fix in 中文说明Triage 重跑再次停在模板关卡——没有提交新的评审，因为首轮留下的 request-changes 评审仍然在拦截本 PR（其 @yiliang114 PR 正文仍然缺少 PR 模板 中同样的三个必填部分：
你对评审发现的回复以及 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-6 branding-verification loop — already reported (comment 3865077236), author responded in findings-response comment 5430094283
- R1-7 zero tests for the script — already reported (comment 3865077241), author declined as out of scope; this round's blockers each name the witness test that would pin them
- R1-8 loadConfig shape validation — already reported (comment 3865077247); re-probed this round (null config and numeric fields throw raw TypeErrors), still stands as discussed
- R1-15 installerHooks untouched — already reported (comment 3865077268), author declined as pre-existing config
- R1-18 native OS surfaces unbranded — already reported (comment 3865077299), author declined as feature enhancement; independently rediscovered this round by four separate audit passes (window title, dialogs, Info.plist strings)
Test Plan (not a blocker): src-tauri/icons/icon.png — no such file or directory; .agents/skills/desktop-brand-builder/scripts/brand-create.mjs — no such file or directory.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:99 — [probe] artifactPrefix override is echoed as applied but never consumedpackages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:100 — [probe] string updaterEndpoints silently coerced to the disable-updates sentinel
— qwen3.8-max via Qwen Code /review (v0.22.2)
…afety - Use function replacers in replaceAll to prevent $& pattern interpretation when appName contains dollar signs (R1-3) - Quote logo path in shell spawn to handle paths with spaces (R1-4) - Add image extension validation for logo files (suggestion R1-6) - Document silent config mutations in SKILL.md (suggestion R1-5) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
1 similar comment
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because GitHub had not computed the PR merge ref after several retries — try again shortly. 中文 — 判定：
|
|
Sandboxed verification: The verification ran, but its result artifact could not be retrieved for publishing, so there is nothing to report here. The run log still has the agent output; re-run 中文 — 判定：
|
|
Triage re-run stopped at the template gate again — the first pass's request-changes review still gates this PR, so no new review was submitted. @yiliang114 Progress first: the last two commits ( The blocker is still only the PR body — the same three required parts of the PR template are missing:
Add those three sections and re-run 中文说明Triage 重跑再次停在模板关卡——首轮留下的 request-changes 评审仍然在拦截本 PR，因此没有提交新评审。 @yiliang114 先说进展：最新的两个提交（ 卡住进度的仍然只是 PR 正文——PR 模板 中同样的三个必填部分缺失：
补齐上述三个小节后重新触发 — Qwen Code · qwen3.8-max Reviewed at |
… re-run - SKILL.md: document that build:runtime must be re-run with QWEN_DESKTOP_TARGET set before each cross-compile target, otherwise the packaged artifact contains a wrong-arch Node binary. - brand-create.mjs: detect already-branded shell-root (productName no longer 'Qwen Code Desktop') and fail early with a clear message, preventing silent stale results from non-idempotent re-runs. - SKILL.md: add explicit 'never re-run in the same clone' warning to Failure Handling section. Addresses review findings R1-16 (cross-compile docs) and idempotency guard from qwen-code-ci-bot. Co-authored-by: Qwen Code <noreply@alibaba-inc.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- Token-scan R1-4 tripwire — already reported (round-4 review body deferral R4-6)
- runBrand configDir temp-dir leak — already reported (round-4 review body deferral R4-4)
- Missing bootstrap/ preflight (brand-create.mjs:236) — already reported (round-4 review body deferral)
- No test reads back the written tauri.conf.json — already reported as R4-2 (round-4 review body deferral)
- loadConfig type-confusion raw TypeError — already reported as R1-7 (comment 3865077247)
Not explored to full depth (tool budget reached): "agent reverse-audit (round 4)": none — no check was cut short by the ceiling (line omitted per brief; nothing was truncated)..
Test Plan (not a blocker): src-tauri/icons/icon.png — no such file or directory; .agents/skills/desktop-brand-builder/scripts/brand-create.mjs — no such file or directory.
Deferred under the convergence posture (round 5, not a blocker) — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s) — recorded, not requested in this round:
scripts/tests/brand-create-safety.test.js:204 — [probe] hostile-appName test pins syntactic validity only, not content fidelityscripts/tests/brand-create-safety.test.js:143 — [probe] one test reaches generateIcons without the CLI stub → npx network fallbackscripts/tests/brand-create-safety.test.js:196 — [probe] index.html branch of patchBootstrap is exercised by no testpackages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:137 (+2 locations) — [probe] config.identifier unvalidated from both appId vectorspackages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:206 — [probe] npx fallback runs unpinned latest @tauri-apps/cliscripts/tests/brand-create-safety.test.js:167 — [probe] detectAlreadyBranded re-run guard has no testpackages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:98 — [probe] updaterEndpoints elements unvalidated, written verbatim
Convergence: round 5 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs (findings in round 4; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already engage the critical posting floor — it resolved early, ahead of the round-6 schedule, because the first-time-finding rate has not fallen for consecutive rounds. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (2 Critical(s)), the rate of first-time findings is not falling (this round 2, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
@yiliang114 The template gate still holds — the code findings are addressed now, but the body is still missing the same three required parts of the PR template:
What I verified on the new commit since the last pass:
Fill in the three sections above and re-run 中文说明模板关卡仍然未通过——代码问题已处理，但 PR 正文仍缺少 PR 模板 中同样三个必填部分：
自上次以来在新提交上核实到的进展：
补齐上述三个部分后重新触发 — Qwen Code · qwen3.8-max Reviewed at |
… closed on missing updater section (R5-1, R5-8) R5-1: appName was spliced raw into bootstrap/index.html (<title>, alt attribute, <h1>, <h2>), so free-form brand names with <, > or " produced broken markup (and a </title> breakout). The index.html path now uses an HTML-escaped form while bootstrap.js keeps its JS-literal escaping. R5-8: when the target shell-root's tauri.conf.json has no plugins.updater section, the brand's validated updaterEndpoints/updaterPubkey were silently discarded while brand-create exited 0 reporting them as applied. Fail closed before any file is written, leaving the config unmutated. Tests: hostile-appName index.html fixture assertions (escaped output, no raw breakout) and a no-plugins.updater shell-root case (non-zero exit, stderr names the missing section, config byte-identical). Both new assertions go red when the corresponding fix is removed. Suite 8/8 green; prettier + eslint clean on both changed files. Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
|
Fixed in 09a1a43 (head now). Verification (as requested): the hostile-appName suite now seeds a |
|
@qwen-code /triage |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 417 passed · 0 failed · 417 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：417 通过 · 0 失败 · 417 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10164 verification (round 7) — feat(desktop-shell): restore the brand builder skill for the Tauri shellVerdict: This is a re-run at an unchanged head: round 6 verified exactly this commit ( 中文摘要 — 判定：✅ 通过（merge-ready · agent 判定）
Previous-finding status (follow-up round, round 6 → round 7, same head)Round 7 re-runs at the identical head round 6 verified (
Central claim + A/BCentral claim of the PR: Cell table — happy path at head ( Cell table — R5-1 A/B at head vs reconstructed round-5 control (
Every head cell additionally asserts: entity decode round-trips to the exact appName at all 4 splice points; all 11 injected Cell table — R5-8 A/B at head vs guard-removed control (
Positive control: with a Cell table — mutation matrix (vacuity + positive control) (
Every mutant site was asserted present exactly once before surgery; every restore was sha256-verified; post-matrix the working-tree script is byte-identical to head ( Carried re-measurements (all at this head): H4 guard matrix 33/33 ( Corrections
FindingsAll carried items re-measured at this head; none is blocking. No new findings this round. F7 (Suggestion, carried, unchanged) — the shell-less npx fallback cannot resolve - : spawnSync(
- 'npx',
+ : spawnSync(
+ process.platform === 'win32' ? 'npx.cmd' : 'npx',F6 (Nit, carried, stands) — non-string config values crash with uncaught TypeErrors instead of M4 (coverage gap, carried, persists): blinding Observations (non-blocking, unchanged):
Not covered
MethodologyEnvironment: Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- unseeded npx fallback in 'accepts updaterEndpoints when updaterPubkey is provided' test — already reported (round-5 review body deferral, test.js:143)
- runBrand configDir temp-dir leak — already reported (round-4 review body deferral R4-4, test.js:74)
- missing bootstrap/ preflight (ENOENT after config mutation, guard-locked half-branded tree) — already reported (round-4 review body deferral, brand-create.mjs:236)
- no test reads back the written tauri.conf.json (updater success path unpinned) — already reported (round-4 review body deferral R4-2, test.js:93)
- R1-4 tripwire is a token scan, not a behavioural pin — already reported (round-4 review body deferral R4-6, test.js:97)
Test Plan (not a blocker): src-tauri/icons/icon.png — no such file or directory; .agents/skills/desktop-brand-builder/scripts/brand-create.mjs — no such file or directory.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:283 — [probe] HTML-escape & replacement pinned by no test input (mutant survives)packages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:177 — [probe] fail-closed guard's pubkey-only arm pinned by no test (mutant survives)packages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:182 — [review] new fail-closed precondition undocumented in SKILL.mdpackages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs:235 — [probe] npx fallback arm unspawnable on Windows; spawn error swallowed
Convergence: round 6 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/desktop-shell/.agents/skills/desktop-brand-builder/scripts/brand-create.mjs (findings in round 5; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 512 passed · 0 failed · 512 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：512 通过 · 0 失败 · 512 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10164 verification (round 8) — feat(desktop-shell): restore the brand builder skill for the Tauri shellVerdict: This is a re-run at an unchanged head: rounds 6 and 7 verified exactly this commit ( 中文摘要 — 判定：✅ 通过（merge-ready · agent 判定）
Previous-finding status (follow-up round, round 7 → round 8, same head)
Central claim + A/BCentral claim of the PR: Cell table — happy path at head ( Cell table — R5-1 A/B at head vs reconstructed round-5 control (
Every head cell additionally asserts: entity decode round-trips to the exact appName at title and alt; all single-quoted literals in the patched Cell table — R5-8 A/B at head vs guard-removed control (
Positive control: with a Cell table — mutation matrix (vacuity + positive control) (
Every mutant site was asserted present exactly once before surgery; every restore was sha256-verified; post-matrix the working-tree script is byte-identical to head ( Carried re-measurements (all at this head): H4 guard matrix 38/38 ( Corrections
FindingsAll carried items re-measured at this head; none is blocking. No new findings this round. F7 (Suggestion, carried, unchanged) — the shell-less npx fallback cannot resolve - : spawnSync(
- 'npx',
+ : spawnSync(
+ process.platform === 'win32' ? 'npx.cmd' : 'npx',F6 (Nit, carried, stands) — non-string config values crash with uncaught TypeErrors instead of M4 (coverage gap, carried, persists — closure now measured): blinding Observations (non-blocking, unchanged):
Not covered
MethodologyEnvironment: Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
A scalar updaterEndpoints (e.g. a string typo of the JSON-array shape documented in SKILL.md) was silently coerced to [] before the updater-safety guards ran, so brand-create exited 0 while shipping the brand with in-app updates permanently disabled. Reject any non-array value with an explicit error naming the expected shape; omitted or [] still disables updates as documented. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Scope
Reviewed brand-create.mjs (377 lines), SKILL.md (222 lines), scripts/tests/brand-create-safety.test.js (330 lines), .gitignore and eslint.config.js changes. This is a pure addition (934 lines, 0 deletions) across 5 files.
Scope exclusions: cross-compile targets not run (no macOS/Windows toolchain); Tauri build pipeline not executed; only static analysis + code tracing against the previous review threads.
CI Status
Test (ubuntu-latest, Node 22.x)— PENDINGDesktop Shell (windows-2022)— PENDINGTest (windows-latest, Node 22.x)— SKIPPEDTest (macos-latest, Node 22.x)— SKIPPEDIntegration Tests (CLI, No Sandbox)— SKIPPEDDesktop Shell (ubuntu-22.04)— success
Platform test jobs are skipped on this fork PR. Windows and macOS behavior is unreviewed.
Cross-check against prior-round Critical findings
I traced each previously-reported Critical finding against the current head (acce1aa4):
| Finding | Status at current head |
|---|---|
R1-3 / R4-1 — apostrophe/backslash/newline in appName producing invalid JS |
Addressed. Current code uses JSON.stringify(brand.appName).slice(1,-1).replace(/'/g, "\\\'"). Traced: "Bob's App\\" → JSON.stringify → "Bob's App\\\\" → slice → Bob's App\\\\ → replace ' → Bob\\'s App\\\\; spliced into 'Starting Bob\\'s App\\\\' — valid single-quoted JS literal. Test brand-create-safety.test.js line 869 covers this case and checks node --check on the output. |
R5-1 — appName spliced raw into bootstrap/index.html |
Addressed. appNameHtmlSafe applies 5-step HTML escaping (&, <, >, ", '). Tested at line 902. |
R6-1 — scalar updaterEndpoints silently coerced to [] |
Addressed. Explicit !Array.isArray(input.updaterEndpoints) check with fail() at loadConfig line 373. Tested at line 791. |
R3-1 — deleting pubkey causes Tauri plugin deserialization failure |
Addressed. Code sets pubkey = '' and the comment explains why (no serde default). |
R5-8 — validated updater config silently discarded when no plugins.updater section |
Addressed. Explicit fail() at patchTauriConfig before any file write. Tested at line 955. |
R3-4 — single-use guard bypassed when derived appName equals pristine default |
Addressed. loadConfig rejects appName === 'Qwen Code Desktop'. Tests cover appName override and brandId-derived case. |
Remaining observations
F1 (minor) — partial-write window in main():
patchTauriConfig writes tauri.conf.json to disk before patchBootstrap is called. If patchBootstrap throws (e.g. copyFileSync on a missing bootstrap/ directory, or an unreadable file), the process exits with an unhandled exception. At that point tauri.conf.json already has productName changed, so detectAlreadyBranded fires on any retry — making the clone unrecoverable without manually resetting tauri.conf.json. The documented workflow (fresh clone per brand) limits exposure in normal use, and the standard Tauri shell ships bootstrap/. This is pre-existing deferred context from earlier rounds; no fix proposed.
F9 (question) — generateIcons npx fallback on Windows without local @tauri-apps/cli:
resolveTauriCli correctly avoids the npx.cmd problem when @tauri-apps/cli is locally installed. The fallback spawnSync('npx', ['--yes', ...]) without shell: true would fail with ENOENT on Windows (Node cannot resolve npx.cmd without shell). This path is reached only when the local resolution fails; npm install --workspaces=false in the workflow should prevent it. Desktop Shell (windows-2022) CI is still running; if it exercises this fallback, it will surface the failure.
What was verified
- Escaping chain for JS string literals (apostrophe, backslash, newline, double quote) — traced end-to-end, matches
node --checktest. - HTML escaping chain for
index.html— traced, confirmed 5 entities covered, test covers<and"cases. updaterEndpointsvalidation path — all branches traced: empty, non-array, non-empty with/without pubkey.- Single-use guard —
detectAlreadyBranded↔loadConfigappName rejection form a consistent pair. - Updater pubkey handling — empty-string sentinel (not deletion) consistent with plugin schema requirement.
.gitignoreaddition forbrand-builds/— consistent with the workflow's$PWD/brand-builds/clone target.eslint.config.jsexclusion — patternpackages/desktop-shell/.agents/skills/**/scripts/**/*.mjscorrectly scopes the Node-globals rule to this skill's scripts.
Reviewed with AI assistance.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Test Plan (not a blocker): src-tauri/icons/icon.png — no such file or directory; .agents/skills/desktop-brand-builder/scripts/brand-create.mjs — no such file or directory.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
scripts/tests/brand-create-safety.test.js:153 — [probe] R6-1 regression test pins only truthy scalars; falsy non-array updaterEndpoints (null/0/false/'') stay unpinned against a future truthiness refactor
中文说明
无阻断问题。LGTM！✅
Test Plan（非阻断）：src-tauri/icons/icon.png — no such file or directory; .agents/skills/desktop-brand-builder/scripts/brand-create.mjs — no such file or directory。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Review findings — 3 medium-priority issues
JavaScript syntax checks pass. The isolated review checkout could not run ESLint because dependencies were not installed. |
|
Released in v0.22.3. |
6 participants
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 withpackages/desktopin #9085. The skill keeps the original contract — a branded desktop package from a minimalbrandId+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), thesrc-tauri/icons/set, and thebootstrap/startup UI. The bundledbrand-create.mjsis dependency-free plain Node (desktop-shell uses npm, not bun), regenerates the full icon set viatauri 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
Confirm: summary JSON reports
appName: "Acme AI",appId: "ai.acme.desktop";/tmp/ds-test/src-tauri/tauri.conf.jsonhas the new productName/identifier and empty updater endpoints;grep -c "Qwen Code" /tmp/ds-test/bootstrap/index.html /tmp/ds-test/bootstrap/bootstrap.jsreturns 0 for both; icons under/tmp/ds-test/src-tauri/icons/are regenerated. Invalid input is rejected: abrandIdofBad_IDexits 1 with the regex message.Evidence (Before & After)
N/A (agent skill + build script, no user-visible UI change). Local run output:
Tested on
Environment (optional)
Node 22, plain
nodeinvocation (no dependencies);tauri iconis used for icon regeneration when available.Risk & Scope
.agents/skills/directory + self-containedbrand-create.mjs); it never runs in the shipped app and only mutates a scratch copy ofpackages/desktop-shellpointed at by--shell-root.Linked Issues
desktop-brand-builderskill).中文说明
这个 PR 做了什么
在
packages/desktop-shell/.agents/skills/desktop-brand-builder/恢复了面向 Tauri 壳的桌面品牌构建技能，替代随 #9085 一起删除的 Electron 时代技能。技能保持原有契约——只需最小的brandId+logo即可产出一个品牌化桌面包——但作用于 Tauri 壳的品牌化挂点而非 Electron 构建树：src-tauri/tauri.conf.json（productName、identifier、shortDescription、updater 端点）、src-tauri/icons/图标集、bootstrap/启动页。自带的brand-create.mjs为无依赖的纯 Node 脚本（desktop-shell 用 npm 而非 bun），通过tauri icon重新生成完整图标集，并默认清空 updater 端点，保证品牌构建既不会轮询官方更新源、也不会被其更新。为什么需要
#9085 的 Electron 移除删掉了唯一的白牌定制路径（
desktop-brand-builder及 #4581 的多品牌支持）。当前 Tauri 壳只服务单一品牌，任何想要自定义桌面客户端的下游（如 OpenWork 当年使用该技能后再 fork 的方式）都没有入口，只能手工重新摸索品牌化挂点。本 PR 在新的架构上恢复这一能力，趁需要改哪些地方的知识尚未流失。这也是 #8092 所定方向的后续。评审者测试计划
如何验证
确认：summary JSON 报告
appName: "Acme AI"、appId: "ai.acme.desktop"；/tmp/ds-test/src-tauri/tauri.conf.json有新的 productName/identifier 且 updater 端点为空；grep -c "Qwen Code" /tmp/ds-test/bootstrap/index.html /tmp/ds-test/bootstrap/bootstrap.js两者均为 0；/tmp/ds-test/src-tauri/icons/下图标已重新生成。非法输入会被拒绝：brandId为Bad_ID时以正则报错信息退出码 1。证据（修改前后）
N/A（agent 技能 + 构建脚本，无用户可见 UI 变化）。本地运行输出见英文正文。
测试环境
环境（可选）
Node 22，纯
node调用（无依赖）；图标重新生成在可用时使用tauri icon。风险与范围
.agents/skills/目录 + 自包含的brand-create.mjs）；它不会随发布的应用运行，只会修改--shell-root指向的临时副本。关联 Issue
desktop-brand-builder技能，本 PR 是其后续恢复。