fix(desktop): harden release pipeline - #9009
Merged
Merged
Conversation
Collaborator
Collaborator
|
Re-run after another merge from main. The head moved from Template — complete ✓ Problem — observed, not theoretical; unchanged from prior passes. Direction — aligned. Desktop release infrastructure hardened against its own observed failure modes. The author is a repo maintainer who has driven several review rounds on it, so the release-surface escalation this kind of change normally triggers is already covered by the people in the thread. Size — +425/−47 across 6 files, identical to the previous pass (two release workflows, two release scripts, their tests, plus a 7-line design doc). No core module paths touched; not applicable. Approach — scope unchanged and still right: five concrete defects handled as one release unit, each traceable to a real failure. The one open thread from the last pass is closed — the PR description was updated to describe the final semantics (recoverable-on-next-run swap, newer stable feed intentionally preserved, OSS hard-fail on real release runs only), so release operators reading it now get the accurate picture. No unrelated changes in the diff. Risk — no elevated risk signals; none of the changed files match the revert-correlated paths. Moving on to code review. 🔍 中文说明本次为又一次合并 main 之后的重跑。head 从 模板 — 完整 ✓ 问题 — 已观测到，不是理论问题；与之前几轮一致。Windows 上 方向 — 对齐。这是针对 Desktop 发布基础设施自身已观测故障模式的加固。作者是仓库 maintainer，并已主导多轮 review，此类改动通常需要的 release 层面上报已由线程中的相关人员覆盖。 规模 — +425/−47，共 6 个文件，与上一轮一致（两个发布 workflow、两个发布脚本、对应测试，外加 7 行设计文档）。未触及核心模块路径，不适用。 方案 — 范围不变且仍然合理：五个具体缺陷作为一个发布单元处理，每个都能追溯到真实故障。上一轮遗留的唯一问题已关闭——PR 描述已更新为最终语义（下次运行可恢复的替换、严格更高的稳定 feed 有意保留、OSS 硬失败仅适用于真实发布运行），发布操作者现在读到的是准确描述。diff 中没有无关改动。 风险 — 无升级风险信号；改动文件未命中任何与 revert 相关的路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewThe head moved to What I verified at
No findings at this head. The previous pass's one nit — the stale PR description — is closed: the description now matches the final semantics. CI evidenceUnattended CI run — nothing was built or executed here; this is the PR's own CI on the reviewed commit, read through the API.
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 On the sandboxed-lane question: same surface as prior passes. What genuinely cannot be settled from outside the real release pipeline — the dispatch gate, live artifact collection during a signed build, and the OSS sync failure paths — has no lane: there is no TUI for 中文说明代码审查：head 移动到 在 本 head 无发现。上一轮唯一的小问题——过时的 PR 描述——已关闭：描述现已与最终语义一致。 CI 证据：本次为无人值守 CI 运行，未构建或执行任何 PR 代码，证据来自 PR 自身 CI（经 API 读取）。 关于沙箱验证：与之前几轮相同的表面。真正无法在发布管线之外定论的——dispatch 门禁、签名构建中的实际产物收集、OSS 同步失败路径——没有任何通道覆盖：没有可供 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — every finding across three review rounds is closed, the last nit (stale PR description) was fixed by the author, and the code at this head is byte-identical to what the previous passes verified; the only open item is the unit job still running, which gates the approval, not the verdict. Stepping back: the new thing this pass was a merge of main that landed #8988's bootstrap-visibility test into the very file this PR extends. That is exactly the interaction that can silently break a long-lived branch, so I diffed the two heads and re-read the full diff at the new one: the PR's five other files are untouched, the merged Verdict: approve — with one precondition. @yiliang114 — nothing left on the code side. The first dry-run prerelease dispatch from your reviewer test plan (stable version rejected, 中文说明回顾：本轮的新情况是一次 main 合并把 #8988 的 bootstrap 可见性测试落进了本 PR 扩展的同一个文件。这正是可能悄悄破坏长期分支的交互，因此我对比了两个 head 的差量并在新 head 上重新通读了完整 diff：本 PR 的其余五个文件未被触碰，合并后的 结论：批准——带一个前置条件。审查时 @yiliang114 — 代码层面已无遗留。你 Reviewer Test Plan 中的首次 dry-run prerelease dispatch（稳定版号被拒、 — Qwen Code · qwen3.8-max Reviewed at |
yiliang114
commented
Aug 12, 2026
yiliang114
commented
Aug 12, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed exact head 7921ae2016fee3492810f47f34764ee1e50deec5 against a32ec1ee4a33323556ee5f9cd2dc66ff0282645b. One P2 interruption-safety issue remains in runtime replacement; see the inline comment. The prerelease gate, verified Node cache, platform artifact allowlists, and same-run OSS feed check otherwise look correct.
Contributor
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
approved these changes
Aug 12, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
yiliang114
enabled auto-merge
August 12, 2026 13:06
chiga0
reviewed
Aug 12, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #9009 fix(desktop): harden release pipeline
Author: @yiliang114 · HEAD: 7921ae2 · Base: a32ec1e
Type: Infra / CI + Bug Fix (release pipeline hardening, 5 independent defects in one unit)
Change size: +284 / -41 across 6 files
CI: all required checks green on 7921ae2; mergeable_state: blocked (pending required approval only)
Findings Summary
- Critical: 0
- Major: 1
- Minor: 4
- Nit: 2
Key Observations
The five changes are individually well-targeted and each maps to a defect that is verifiable in the shipped release history. The staging-plus-atomic-swap rework of prepare-runtime.js is the right mechanism, and the per-OS artifact allowlists are a genuine correctness fix (the old recursive find could also sweep files out of the Linux *.AppDir tree, not just rg.exe).
My one substantive disagreement is with the Node.js archive cache. The existing automated review states the "trust anchor is unchanged (nodejs.org's SHASUMS256 on first fetch)". That is not what the code does: on a cache hit both the archive and the SHASUMS256.txt it is verified against are read back from the same cache directory, whose default is the fixed, world-writable path os.tmpdir()/qwen-desktop-node-cache. Verification becomes self-referential, so the trust anchor moves from nodejs.org to the local filesystem. See the inline comment on prepare-runtime.js:161.
Notably, this repository already implements the correct pattern one file away: sync-desktop-to-oss.yml:124 pins OSSUTIL_SHA256 out-of-band and checks the download against it, precisely so that a downloaded artifact can never supply its own expected hash.
Industry Context
Content-addressed toolchain caches consistently keep the integrity reference outside the cache: Go's go.sum, Nix's output hashes, and Bazel's sha256 attribute on http_archive all require the expected digest to come from a source the cache cannot influence. nvm and actions/setup-node likewise re-fetch SHASUMS256.txt per install rather than storing it next to the cached tarball. Pinning the expected SHA-256 in the repo (as this repo already does for ossutil), or always re-downloading the ~3 KB SHASUMS256.txt, would restore that property at negligible cost.
On the CI side: actions/cache@v4 declares post-if: success(), so the entry is only written when the entire build job (Rust compile, codesign, notarization, smoke tests) succeeds — the retry-after-a-flaky-download case the cache is meant to help is exactly the case where nothing gets saved. Combined with GitHub's 7-day idle eviction and the 10 GB per-repo LRU cap, a workflow_dispatch-only workflow will usually see a cold cache.
Cross-Validation
| Finding | Other Reviewer | My Assessment |
|---|---|---|
replaceRuntime() interruption window between the two renames strands previous and leaves no canonical runtime |
@yiliang114 (inline, prepare-runtime.js:345) |
Confirmed — independently found in my Round 2 audit. Verified at HEAD lines 342-355: a signal between line 345 and line 348 bypasses both the catch and the finally. Extended below: the same gap also leaks the whole staging tree, and nothing sweeps it on the next run. |
| Node cache: "every use re-verifies against the cached SHASUMS256.txt … the trust anchor is unchanged" | qwen-code-ci-bot (APPROVED, stage=2) | Disagree — the re-verification uses the cached manifest (lines 160-182), so it is self-consistent by construction. The default cache root is a predictable world-writable /tmp path. See Major-1. |
-mindepth 2 -maxdepth 2 is "belt-and-braces; Tauri lays artifacts at bundle/<format>/<file>" |
qwen-code-ci-bot | Confirmed for today's layout — also verified the bundle/electron-bridge/*.zip path added by the bridge step (desktop-release.yml:448) is depth 2. Partial caveat: the only guard is a totally empty destination (line 517); a per-platform expected-count assertion would make a future layout change fail loudly rather than silently. |
OSS gate: "SOURCE wired from validated steps.release.outputs.source; matches stated semantics exactly" |
qwen-code-ci-bot | Confirmed for the lag direction. Incomplete in the other direction — see Minor-3. |
| 120 s uncached download timeout is out of scope / non-blocking | qwen-code-ci-bot | Agree it is out of scope, but the mitigation is weaker than assumed — see the post-if: success() note above and Minor-2. |
| CI cache save semantics and eviction economics | — | Unique-Mine — desktop-release.yml:182. |
OSS feed can still be regressed by a clobber re-run of an older version |
— | Unique-Mine — sync-desktop-to-oss.yml:195. |
Stale .prepare-* staging trees are never swept |
— | Unique-Mine — prepare-runtime.js:31. |
Test hardcodes .nvmrc as 22 while deriving the archive name from process.versions.node |
— | Unique-Mine — test-release.js:276. |
Additional Audit Coverage
Areas I independently checked that produced no finding, recorded so the audit depth is visible:
- Caller/consumer impact —
prepare-runtime.jsis reached frombuild:runtime(package.json) and fromdesktop-release.yml:383(--refresh-checksumsafter macOS signing). The--refresh-checksumsbranch now defaults tofinalPackageRootand short-circuits beforestagingRootis created (lines 21-31), so the post-signing refresh still targetsruntime/qwen-code. No regression. - TDZ check —
writeChecksums(root = packageRoot)is declared at line 308 but called at line 26, beforeconst packageRoot(line 32). Safe only because the--refresh-checksumsbranch always passes an explicitroot, so the default initializer is never evaluated. Correct, but load-bearing and undocumented. - Bundling blast radius —
tauri.conf.jsonmapsbundle.resourcesto../runtime/qwen-code(notruntime/), so a leaked.prepare-*directory cannot be shipped inside an installer. It is also covered byruntime/*inpackages/desktop-shell/.gitignore, hence invisible rather than harmless. - Artifact completeness —
create-desktop-update-manifest.mjsenforcesmatches.length !== 1per platform and throws on a missing.sig, so the four updater artifacts fail loudly if the new allowlists ever drop one..dmg/.debcompleteness is only checked insync-desktop-to-oss.yml:102-110, i.e. for stable releases only. - Prerelease gate composition — combined with the pre-existing SemVer regex at
desktop-release.yml:81, the new gate correctly rejects0.2.1,0.2.1-and0.2.1+foowhile accepting0.2.1-rc.1. Residual asymmetry:prerelease=false, draft=truestill accepts0.2.1-rc.1, and such a draft, once published manually, becomes a stable-flagged release carrying a prerelease version thatsync-desktop-to-oss.yml:58would then refuse to mirror. Narrow, not raised as a finding. - Cross-compilation / cache keying — the
macOS x64matrix leg cross-compiles on an arm64macos-15runner; the cache key includesmatrix.rust_targetand the archive name embeds the Node target, so the two legs cannot collide. - Project conventions —
docs/design/<slug>.mdwithout a date prefix matches the dominant existing style (including the siblingdesktop-electron-to-tauri-update-bridge.md). AGENTS.md's core-infrastructure gate does not apply (nopackages/core/**paths). CONTRIBUTING.md §1 (linked issue) is not satisfied — the PR states "No linked issue" — noted only for maintainer awareness. - Concurrency — two local builds for different targets share
cacheRoot/v<ver>/SHASUMS256.txt;copyFileSyncis not atomic, so a mid-copy read can surface as a hardNode checksums do not list …failure at line 178 rather than a graceful re-download. Writing to a temp name andrename-ing would remove this. Folded into Major-1.
Final Verdict
Comment — merge after addressing Major-1. The design is sound and I found no simpler alternative for any of the five fixes. The one change I would not ship as-is is the Node.js archive cache: it silently converts a network-anchored integrity check into a self-referential one rooted in a predictable world-writable directory, and the existing approving review explicitly (and incorrectly) certifies the opposite. The remaining items are small and can land as follow-ups. @yiliang114's own inline note on replaceRuntime() is valid and I independently reached the same conclusion.
This review was generated by QoderWork AI
yiliang114
requested review from
qwen-code-review-bot
and removed request for
qwen-code-review-bot
August 12, 2026 13:23
qwen-code-ci-bot
previously requested changes
Aug 12, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #9009 hardens the Desktop release pipeline: it require...: none — all planned checks completed within budget.; PR #9009 hardens the Desktop release pipeline: it require...: none — all checks above completed within budget..
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/sync-desktop-to-oss.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未探索到全部深度（达到工具调用预算）：PR #9009 hardens the Desktop release pipeline: it require...：none — all planned checks completed within budget.；PR #9009 hardens the Desktop release pipeline: it require...：none — all checks above completed within budget.。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/sync-desktop-to-oss.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.10)
yiliang114
commented
Aug 12, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
The current code paths look clean in this pass. One non-blocking follow-up: the PR description is stale in two places. Runtime replacement is recoverable on the next preparation run rather than strictly atomic, and a strictly newer stable feed is intentionally accepted instead of treating every version mismatch as a failure. The description and reviewer test plan should match those final semantics before merge.
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
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: 255 passed · 0 failed · 255 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：255 通过 · 0 失败 · 255 总计 Verification reportPR 9009 Deep Verification —
|
| Cell | Observable oracle | base 50097c15 |
head 895ca61a |
|---|---|---|---|
| C1 fresh run | exit 0; runtime + checksums.json assembled; 1 archive + 1 SHASUMS fetch |
✅ (no cache written) | ✅ + cache archive byte-identical to canonical sha256 |
| C2 warm run | archive fetches in run 2 / cache-hit line | re-downloads (1 fetch) | 0 fetches, Using cached Node.js runtime … |
| C3 poisoned cache | tampered archive + attacker-planted SHASUMS256.txt in cache dir |
n/a (no cache) | rejected: re-download, planted file removed, canonical archive repopulated |
| C4 download 500, existing runtime present | previous runtime content (marker) | destroyed before download; half-assembled runtime left | intact; no stranded staging |
| C5 interrupted swap (crash between renames) + next run fails | previous runtime | lost; incomplete runtime left | recovered and preserved; stale staging swept |
| C6 interrupted swap + next run succeeds | final state | new runtime (old silently discarded) | new runtime, no stranded .prepare-* |
C4/C5 are the motivating bug, reproduced on base exactly as the description claims ("a timeout occurred after the runtime directory had already been deleted"). Note base leaves a partial qwen-code/ (lib copied, node/manifest absent in C4; LICENSE/manifest absent in C5) — the "半成品油" state.
Replay: workflow bash steps (82/82 — 02-replay-workflow-steps-base-vs-head.png)
| Group | Flips proven (base → head) |
|---|---|
R1 Resolve version (8 input vectors × 2 arms) |
0.2.1 + prerelease=true: accepted on base → rejected on head (Desktop prereleases must use a SemVer prerelease suffix); also for dry_run=true. 0.2.1-rc.1 passes both; stable-publish X.Y.Z rule, v-prefix strip, invalid-SemVer rejection unchanged. |
R2 Collect artifacts (Windows/Linux/macOS trees × 2 arms) |
base leaks rg.exe (depth 2 and 3), stray.exe, uninstaller-helper.exe on Windows; stray.deb (depth 1) on Linux; depth-1 foo.app.tar.gz on macOS. Head collects exactly the allowlisted installer+sig set on all three platforms; the legit set is still collected on base (control). |
R3 Update stable updater feed (6 feed states × 2 arms) |
feed newer (0.3.0) while publishing 0.2.1: base uploads (downgrade) → head exits 0, no upload, notice; feed invalid: base overwrites → head exits 1; feed release missing the manifest asset: base overwrites → head fails loud; behind/same/absent feed: identical create/upload behavior both arms. |
R4 sync Check whether release matches GitHub stable feed (6 states × 2 arms) |
lagging feed + SOURCE=artifact: base silent matches=false → head exits 1 (GitHub stable feed is 0.2.0 after publishing Desktop 0.2.1); newer feed tolerated both arms (head adds the explicit notice); manual backfill (SOURCE=release) stays non-fatal even when the feed lags or is corrupt. |
Supporting boundary probe: sort -V (used by both feed guards) orders multi-digit components correctly (0.10.0 > 0.9.0, 0.10.0 > 0.2.1) — 2/2 scripted checks.
The concurrency-group change (desktop-release-${{ inputs.dry_run && inputs.version || 'publish' }}) serializes all publish runs into one group while dry-runs stay per-version; combined with cancel-in-progress: false this makes the feed's read-compare-write race-free within the repo. Validated by actionlint; the expression's precedence (&& before ||) yields the intended grouping.
Corrections
None needed — no earlier review round on this PR, and the description matched observed behavior on every probe.
Findings (none blocking)
1. (Low) The new workflow tests pin YAML text, not behavior — three surviving mutants demonstrate it
testDesktopReleaseHardening and desktop-oss-workflow.test.js assert substrings/regexes over the workflow files. Mutants that preserve the asserted text but break the semantics pass the entire PR suite and are caught only by behavioral replay:
| Mutant | PR suite | Behavioral replay |
|---|---|---|
M2b: Windows case pattern → *-setup.exe|*-setup.exe.sig|*) ;; (catch-all re-accepts everything) |
✅ passes (Desktop release helper checks passed.) |
❌ caught — rg.exe + uninstaller-helper.exe leak again |
M5: drop -mindepth 2 -maxdepth 2 from the artifact find |
✅ passes | ❌ caught — Linux leaks depth-1 stray.deb (its allowlist matches any *.deb), macOS leaks depth-1 *.app.tar.gz; Windows still saved by the case allowlist alone |
M3: replaceRuntime(); writeChecksums(finalPackageRoot); (checksums written after the swap) |
✅ passes — the ordering assert indexOf('replaceRuntime();') > indexOf('writeChecksums();') compares against −1 because the literal writeChecksums(); no longer exists |
not pinned by any behavioral test either (a post-run existence check can't see the window) |
The killed mutants confirm the suite's positive power (M1 prerelease gate, M2 allowlist removal, M4 recoverInterruptedRuntime() removal, M6 SOURCE='artifact' branch — all fail with the intended assertion; matrix evidence 03-mutation-matrix-live.png, 28/28). The shipped code itself is correct — this finding is strictly about the regression tests' strength: a future one-character semantic edit can pass the PR's whole release-helper + OSS-workflow suite. No code change required for merge; if hardened later, the replay harness in this artifact (harness/replay-workflow-steps.mjs) is a drop-in behavioral oracle.
2. (Low) Node-cache key includes ${{ inputs.dry_run }}, splitting dry-run and publish caches
Actual key: desktop-node-v2-<rust_target>-<NODE_VERSION>-<dry_run>; the description states the key is "per target and Node version". A dry-run's verified archive is never reused by the publish run for the same version (and vice versa), so each lane pays one extra nodejs.org download per (target, version), and two cache entries are stored. No correctness impact — restored archives are always re-verified against a freshly downloaded SHASUMS256.txt (proven by C3) — and branch scoping of actions/cache already isolates fork/branch dispatches, so the extra dimension buys no safety. Consider dropping it; if intentional, the description's risk note should say so.
3. (Low) Cache invalidation is unobservable
copyValidCachedArchive catches the checksum failure silently (catch {}) and falls back to downloading; nothing logs why the cache was not used (observed in C3's stdout: no invalidation message, only the final Prepared desktop runtime …). A persistently corrupt cache directory would re-download on every build with no signal in CI logs. One console.warn on the invalidation path would carry the reason. (The accept path is fully verified: C3 proves the poisoned archive is never extracted and the canonical archive repopulates.)
Behavior changes worth operator awareness (not defects)
- A
desktop-latestfeed release that exists withoutdesktop-latest.jsonnow hard-fails the publish feed step (R3/f6; base overwrote silently). This is the intended fail-loud direction, but a manually broken feed release blocks stable publishing until repaired. - Dry-run prereleases are also gated by the suffix rule (R1/v7) — consistent with the description's "prerelease dispatches require a suffix".
Not covered
- Per-commit attribution: the metadata lists 3 commits (
7921ae20,a1f78f72merge-from-main,895ca61a), but the depth-2 checkout reaches only the head tip (git rev-list HEAD^1..HEAD^2→ 1, both parents grafted/shallow). Verified the aggregateHEAD^1..HEADdiff, which is what lands. - yamllint: could not run — the image has no
pip3/yamllint module (scripts/lint.js --setup:pip3: Permission denied). actionlint, shellcheck, andbash -nall ran and were proven live (see Methodology). scripts/tests/install-script.test.jscollection failure in the scripts suite: throws at import because the container shipsunzipbut notzip; the file is untouched by this PR (emptyHEAD^1..HEADdiff) and its guard text names the condition. Remaining suite: 52/53 files, 1036/1036 tests pass;desktop-oss-workflow.test.js6/6.- Real network/registry surfaces: nodejs.org downloads mocked at the fetch layer (archive + checksum fixtures; the author separately reports the pre-existing 120 s timeout on real uncached downloads). Real GitHub release/OSS APIs replaced by a state-driven
ghstub — the replays reproduce the decision logic's shape (exact scripts, exact env, realjq/sort), not live API effects; calibration against a real pastUpdate stable updater feedrun log was not available (no token by design). - Real Tauri builds on Windows/macOS/Linux runners and the true on-disk location of
rg.exeinside an NSIS bundle: R2 used synthetic bundle trees shaped like tauri's output, with decoys placed at the worst depths (1, 2, 3) so the allowlist — not just the depth flags — had to do the work. ELECTRON_BRIDGE=truefeed variant (asset list unchanged by this PR), desktop-shellcargo test(no Rust changes), repo-wide gates (full suite/lint/typecheck are covered by the PR's own CI).
Methodology
Environment: node:22-bookworm container (Node v22.23.2), merge-ref checkout at depth 2; npm ci + npm run build pre-run. Base arm is a scratch worktree at HEAD^1 rebuilt nowhere — prepare-runtime.js is plain JS with node: builtins only, so no workspace links are crossed; each cell instead copies the arm's script into an isolated fixture tree (repo/pkg/scripts/prepare-runtime.js, fake source via QWEN_CODE_ROOT, QWEN_DESKTOP_SKIP_BUILD=1), eliminating any head-code bleed into base cells. Harnesses live in harness/ (ab-prepare-runtime.mjs, fetch-mock.mjs, replay-workflow-steps.mjs, mutation-matrix.mjs); per-cell stdout/stderr in logs/, gh call logs and extracted step scripts under replay/sandboxes/. Assertion ledger: A/B harness 41 + replay 82 + mutation matrix 28 + gates 104 (bash -n ×46, shellcheck ×46, actionlint ×1, liveness probes ×2, sort -V probes ×2, vitest desktop-oss ×6, test-release.js positive control ×1) = 255, all scripted, all executed; fail counts only unexpected outcomes, of which there were none. Gates proven live before citation: a planted needs: error was caught by actionlint and a planted unquoted expansion by shellcheck. Mutations were applied as exact single-occurrence replacements and every file restored byte-identical (git diff --exit-code asserted per mutation; final git status clean).
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
dismissed
their stale review
August 12, 2026 15:38
Superseded: this request-changes review was submitted on 7921ae2 by a /review run that hit its tool budget before full depth. The follow-up commit 895ca61 addresses every finding it listed (Critical R1-1 closed via dry-run/publish cache-key split plus fresh-SHASUMS re-verification on every use; R1-4–R1-13 mapped in the triage stage-2 comment). Full re-review at 895ca61 found no blockers.
Collaborator
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
yiliang114
requested review from
qwen-code-review-bot
and removed request for
qwen-code-review-bot
August 12, 2026 15:43
Collaborator
|
The release-hardening flow looks coherent overall, but I found one actionable issue:
I did not find a remaining blocker in the runtime recovery, cache checksum verification, or artifact filtering paths. |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
Author
|
@qwen-code-review-bot review this pr and approve if no blockers |
Collaborator
|
No code blockers on the current head. The stricter SemVer validation remains a non-blocking follow-up, but CI is still running on |
Collaborator
|
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: 234 passed · 0 failed · 234 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：234 通过 · 0 失败 · 234 总计 Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) - follow-up round at head Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, mutation matrix, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 234 passed · 0 failed · 234 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）· 跟进轮沙箱验证在隔离、无凭证的容器中重新执行了该 PR 的全部关键测量（与 base 构建 A/B 对照、无 mock harness 断言、突变矩阵、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：234 通过 · 0 失败 · 234 总计
Verification reportPR 9009 Deep Verification (follow-up round) —
|
| # | Finding (previous round) | Severity | Status at new head | Re-measurement |
|---|---|---|---|---|
| 1 | New workflow tests pin YAML text, not behavior — surviving mutants M2b (Windows case catch-all |*), M3 (checksums written after swap), M5 (drop -mindepth 2 -maxdepth 2) pass the whole PR suite |
Low | stands | Mutation matrix re-run: M2b and M5 pass test-release.js + desktop-oss-workflow vitest and are caught only by the behavioral replay (rg.exe/uninstaller-helper.exe and depth-1 stray.deb/foo.app.tar.gz leak again); M3 passes all oracles because indexOf('writeChecksums();') compares against −1 and no post-run oracle sees the swap window. 03-mutation-matrix-live.png |
| 2 | Node-cache key includes ${{ inputs.dry_run }}, splitting dry-run/publish caches, while the description says the key is per target + Node version |
Low | stands | Key unchanged at both sites (desktop-release.yml L194 restore / L348 save): desktop-node-v2-${{ matrix.rust_target }}-${{ env.NODE_VERSION }}-${{ inputs.dry_run }}. Mechanism identity re-verified: restore and save share the same steps.node-cache-path.outputs.path ($RUNNER_TEMP/qwen-desktop-node-cache), same job/runner, save-on-miss after Prepare bundled runtime, rust_target 1:1 with runner OS — so the cache can hit; the dry_run dimension only costs one extra nodejs.org download per (target, version) per mode. No correctness impact (restored archives re-verified, C3). |
| 3 | Cache invalidation is unobservable: copyValidCachedArchive swallows the checksum failure in catch {} with no log line |
Low | stands | C3 cell re-run: poisoned cache rejected, canonical archive repopulated, and stdout/stderr contain no invalidation/mismatch/warn line (HEAD/C3 FINDING3 cache invalidation is silent assertion PASS). |
| — | Operator-awareness: a desktop-latest feed release existing without desktop-latest.json now hard-fails the publish feed step (base overwrote silently) |
note | stands | R3/f5: head exits non-zero when the manifest asset is missing; base uploads. |
| — | Operator-awareness: dry-run prereleases are also gated by the suffix rule | note | stands | R1/v7: 0.2.1 + prerelease=true + dry_run=true rejected on head, accepted on base. |
No declined or deferred rows existed; I agree with the previous round's non-blocking classification of all three — the shipped code is behaviorally correct (proven by the A/B and replay), and each finding is about test strength, description accuracy, or observability.
Central claim and A/B proof
Central claim: the bundled desktop runtime is replaced only after a complete, checksummed assembly; the previous runtime survives any download/assembly failure or interrupted swap (recovered on the next run); verified Node.js archives are cached and reused; and the release gates (prerelease suffix, platform artifact allowlists, stable-feed consistency) hold.
All cells ran the real code from each arm: prepare-runtime.js copied from the head tree / base worktree into isolated fixture trees (the script uses only node: builtins, so no workspace links are crossed), and workflow run: blocks extracted verbatim (js-yaml) from each arm's YAML, executed under bash --noprofile --norc with the step's own env, stubbing only gh (state-driven stub with a call log; jq/sort/find/tar are the real container binaries). nodejs.org is mocked at the fetch layer only (local archive + SHASUMS256.txt fixtures). Expected base failures are encoded as assertions, so fail=0 everywhere.
A/B: prepare-runtime.js (49/49 — 01-ab-prepare-runtime-base-vs-head.png, live re-run)
| Cell | Observable oracle | base 50097c15 |
head d82a9e58 |
|---|---|---|---|
| C1 fresh run | exit 0; runtime + checksums.json assembled; 1 SHASUMS + 1 archive fetch |
✅ (no cache written) | ✅ + cache archive byte-identical to canonical |
| C2 warm run | archive fetches in run 2 / cache-hit line | re-downloads (1 fetch) | 0 fetches, Using cached Node.js runtime … |
| C3 poisoned cache | tampered archive + attacker-planted SHASUMS256.txt in cache dir |
n/a (no cache) | rejected: re-download, planted file removed, canonical archive repopulated; no log line explains why (finding 3) |
| C4 download 500, existing runtime | previous runtime marker | destroyed before download; half-assembled runtime left (lib present, node absent) | intact; checksums intact; no stranded staging |
| C5 interrupted swap + next run fails | previous runtime | lost; incomplete runtime left | recovered and preserved; stale .prepare-* swept |
| C6 interrupted swap + next run succeeds | final state | new runtime (old silently discarded) | new runtime, no stranded .prepare-* |
Replay: workflow bash steps (54/54 — 02-replay-workflow-steps-base-vs-head.png, live re-run)
| Group | Flips proven (base → head) |
|---|---|
R1 Resolve version (9 vectors × 2 arms) |
0.2.1/v0.2.1 + prerelease=true (incl. dry_run=true): accepted on base → rejected on head (Desktop prereleases must use a SemVer prerelease suffix); 0.2.1-rc.1 passes both with correct version=/tag= outputs; stable X.Y.Z rule, invalid-SemVer rejection, and Electron-bridge floor unchanged both arms |
R2 Collect artifacts (Win/Linux/macOS trees × 2 arms, decoys at depths 1–3) |
base leaks rg.exe (d2), uninstaller-helper.exe (d2), stray.exe (d3) on Windows; stray.deb (d1) on Linux; depth-1 zip + renamed foo-…app.tar.gz on macOS. Head collects exactly the allowlisted installer+sig set on all three platforms |
R3 Update stable updater feed (7 feed states × 2 arms) |
feed newer (0.3.0, and multi-digit 0.10.0 vs publishing 0.9.0): base uploads (downgrade) → head exits 0, notice, no upload; feed invalid: base overwrites → head exits 1; feed release missing the manifest: base uploads → head fails loud; same/behind/absent: identical upload/create behavior both arms |
R4 sync Check whether release matches GitHub stable feed (6 states × 2 arms) |
lagging feed + SOURCE=artifact: base silent matches=false → head exits 1 (GitHub stable feed is 0.2.0 after publishing Desktop 0.2.1); invalid feed + artifact: head exits 1; newer feed tolerated both arms (head adds the explicit notice); manual backfill (SOURCE=release) stays non-fatal both arms |
Supporting probes: sort -V orders multi-digit components correctly (0.10.0 > 0.9.0, 0.10.0 > 0.2.1) — 2/2.
Mutation matrix (36/36 — 03-mutation-matrix-live.png, capture of the execution log)
| Mutant | test-release.js | vitest desktop-oss | A/B harness | replay | read |
|---|---|---|---|---|---|
| M1 remove prerelease gate | ❌ killed (prerelease builds must not reuse a stable Desktop version) |
❌ killed | — | ❌ killed | suite+behavior |
| M2 remove Windows allowlist | ❌ killed (Windows release collection must allow only installer executables) |
✅ | — | ❌ killed | suite |
M2b Windows case |* catch-all |
✅ survives | ✅ | — | ❌ killed (rg.exe + uninstaller-helper.exe leak) | behavior only |
| M3 checksums after swap | ✅ survives (indexOf vs −1) | ✅ | ✅ survives | ✅ | true survivor |
M4 remove recoverInterruptedRuntime() |
❌ killed (ENOENT on complete-marker) |
✅ | ❌ killed (C5) | — | suite+behavior |
M5 drop -mindepth 2 -maxdepth 2 |
✅ survives | ✅ | — | ❌ killed (depth-1 leaks) | behavior only |
M6 remove SOURCE=artifact branch |
✅ | ❌ killed | — | ❌ killed | suite+behavior |
| M7 cache always rejected | ❌ killed (/Using cached Node\.js runtime/ mismatch) |
✅ | ❌ killed (C2) | — | suite+behavior |
Unmutated controls green (test-release.js, vitest 6/6, A/B 49/49, replay 54/54). The killed rows are the positive controls proving the suite and oracles can fail; every mutant file restored byte-identical (sha256-verified per mutant; final git status clean). M3 is classified as a coverage gap (the ordering guarantee exists in shipped code and the final state is correct; nothing asserts checksums-before-rename against a concurrent reader), M2b/M5 as test-strength gaps caught by the behavioral oracle in this artifact.
Corrections
None. The previous round's description of the code matched my re-measurements on every probe; no earlier statement needed correcting.
Findings (none new, none blocking)
The three low-severity findings carried from the previous round all stand — see the status table above for the re-measurement evidence. Reproduction commands, in case a reviewer wants to re-run them from this artifact:
- Test-strength survivors:
REPO_ROOT=… ART_DIR=… node harness/mutation-matrix.mjs— inspect the M2b/M3/M5 rows (PR suite exit 0 where the behavioral oracle exits 1). - Cache key:
grep -n "desktop-node-v2-" .github/workflows/desktop-release.yml— both lines carry-${{ inputs.dry_run }}. - Silent invalidation:
REPO_ROOT=… ART_DIR=… node harness/ab-prepare-runtime.mjs— theHEAD/C3 FINDING3 …line shows stdout contains onlyPrepared desktop runtime …after rejecting a poisoned cache.
Not covered
- Per-commit attribution: metadata lists 4 commits (
7921ae20,a1f78f72,895ca61a,d82a9e58c5); the depth-2 checkout reaches only the head tip (git rev-list HEAD^1..HEAD^2→ 1, parents grafted). Verified the aggregateHEAD^1..HEADdiff, which is what lands. - shellcheck / actionlint / yamllint: container has no network (
curl https://github.com→ connection died;node scripts/lint.js --setupfailed to download actionlint). bash -n ran instead and was proven live (planted broken script rejected, exit 2); 4 PowerShell steps (Import Windows certificate,Verify Windows signature, pre-existing on both arms) recognized and excluded from bash -n. actions/cachepinned SHA → tag resolution (0057852b…"# v4"): needs the GitHub API; unavailable here. The mechanism-identity check (same path/key both sides, same job) was done from the YAML instead.- Real network/registry surfaces: nodejs.org mocked at the fetch layer; GitHub release/OSS APIs replaced by the state-driven
ghstub. Replays reproduce the decision logic's shape (exact scripts, exact env, realjq/sort/find/tar), not live API effects. - Real Tauri builds on Windows/macOS/Linux runners: R2 used synthetic bundle trees shaped like tauri output with decoys at the worst depths (1, 2, 3).
scripts/tests/install-script.test.js: collection fails because the container shipsunzipbut notzip(its own guard throws under CI). File untouched by this PR; remaining suite 52/53 files, 1036/1036 tests pass — byte-identical situation to the previous round.ELECTRON_BRIDGE=truefeed asset list (unchanged by this PR), desktop-shellcargo test(no Rust changes), repo-wide gates (covered by the PR's own CI).
Methodology
Environment: node:22-bookworm container (Node v22.23.2), merge-ref checkout at depth 2; npm ci + npm run build pre-run at HEAD; no network. Base arm: scratch worktree at HEAD^1 (tmp/base-tree); prepare-runtime.js cells copy each arm's script into isolated fixture trees (fake source via QWEN_CODE_ROOT, QWEN_DESKTOP_SKIP_BUILD=1, QWEN_DESKTOP_TARGET=darwin-arm64, fetch mocked via NODE_OPTIONS --import), eliminating head-code bleed into base cells. Workflow cells extract run: blocks verbatim with js-yaml, substitute only the documented ${{ }} expressions per cell (unsubstituted leftovers abort the harness), and execute under bash --noprofile --norc with a state-driven gh stub (harness/gh-stub.mjs) and real jq/sort/find/tar. Harnesses: harness/{ab-prepare-runtime,replay-workflow-steps,mutation-matrix,gate-bash-n}.mjs; per-cell stdout/stderr and ledgers in logs/ (pristine ledgers preserved as *-pristine.json); extracted step scripts under replay/sandboxes/. Assertion ledger: A/B 49 + replay 54 + matrix 36 + gates 95 (bash -n 83, non-bash-shell recognition 4, bash -n liveness 1, vitest desktop-oss 6, test-release.js pristine 1) = 234, all scripted, all executed; fail counts only unexpected outcomes, of which there were none. Evidence: 01/02 are live re-runs captured through scripts/verify-capture.mjs; 03 is a capture of the matrix execution log.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
previously approved these changes
Aug 12, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qqqys
approved these changes
Aug 12, 2026
Collaborator
Author
|
@qwen-code /triage |
yiliang114
commented
Aug 12, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
Author
There was a problem hiding this comment.
The current code paths look clean at this head. I left one non-blocking documentation follow-up inline so the design note matches the implemented newer-feed behavior.
|
|
||
| The Desktop release should preserve the last complete bundled runtime until a replacement is fully assembled, recover that runtime on the next run if a swap is interrupted, reuse a Node.js archive verified against a fresh official checksum, and publish only installer/updater artifacts. Published prereleases must use a SemVer prerelease suffix so a later stable build has a strictly newer updater version. | ||
|
|
||
| Stable releases continue to mirror versioned assets to Aliyun OSS before advancing the OSS latest manifest. A normal release run now fails if the GitHub stable feed does not match the version it just published; manual backfills of older releases still leave the latest feed unchanged. |
Collaborator
Author
There was a problem hiding this comment.
[P2] Keep the design contract aligned with the newer-feed branch
This says a normal release fails whenever the GitHub stable feed differs from the version it just published, but the implemented contract intentionally succeeds when the current feed is strictly newer (and the PR description now says so). Could we qualify this as failing only when the feed is invalid or older, while preserving a newer feed? Otherwise the design note tells a future maintainer to undo the intended behavior.
Collaborator
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Collaborator
|
Sandboxed verification: Skipped because the PR is not open for verification (state=MERGED, draft=false). 中文 — 判定：
|
Collaborator
|
Released in v0.21.11. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Hardens the Desktop release pipeline as one cohesive release unit. Prerelease dispatches now require an actual SemVer prerelease suffix, bundled Node.js archives are checksum-verified and cached across builds, runtime assembly replaces the previous runtime only after the new one is complete (and an interrupted swap is recovered on the next preparation run), and release artifact collection excludes embedded executables such as
rg.exe.Stable release-to-OSS synchronization now fails when the GitHub stable feed is invalid or behind the version that was just published. If the feed already points to a newer stable version, the older release keeps both GitHub and OSS latest feeds unchanged.
Why it is needed
A GitHub prerelease marked as
0.2.0is not newer than an installed stable0.2.0, so those users cannot later auto-update to another build with the same version. Local and CI Desktop builds also downloaded Node.js on every run; a timeout occurred after the runtime directory had already been deleted, which allowed an incomplete runtime to be packaged manually. The broad Windows artifact search also published the bundledrg.exeas a standalone release asset.Reviewer Test Plan
How to verify
0.2.1; metadata preparation should reject it.0.2.1-rc.1should pass that gate.*-setup.exeand its updater signature are accepted.Evidence (Before & After)
Before: prereleases could reuse stable versions, runtime preparation deleted the existing runtime before a network download, Node.js was downloaded for every build, and recursive artifact collection could publish embedded executables.
After: prereleases have an upgrade-safe version, verified Node archives are reused, runtime replacement begins only after checksums are written and an interrupted swap is recovered on the next run, release assets use platform-specific installer allowlists, and the OSS feed cannot silently lag or overwrite a newer stable release.
Tested on
Environment (optional)
macOS arm64, Node.js 22.22.0. Targeted ESLint, Desktop release helpers, 23 release/OSS script tests, YAML parsing, diff checks, and
npm run build -- --cli-onlypassed with a clean dependency install. A real uncached Node.js download still reproduced the existing 120-second network timeout; the executable regression covers cache hits and preservation of the previous runtime on assembly failure.Risk & Scope
0.2.1-rc.1whenprereleaseis enabled.Linked Issues
No linked issue.
中文说明
本 PR 做了什么
将剩余的 Desktop 发布问题作为一个完整单元收口。预发任务必须使用真正带后缀的 SemVer 版本；打包使用的 Node.js 归档会校验并跨构建缓存；新的 runtime 只有在完整组装并生成校验文件后才替换旧版本（替换中断时会在下一次准备运行时恢复）；发布产物按平台使用安装包白名单，不再把内置的
rg.exe当作独立产物上传。稳定版本同步 OSS 时，如果 GitHub stable feed 无效或落后于刚发布的版本，任务会直接失败；如果 feed 已经指向更高的稳定版本，则保留 GitHub 和 OSS 的 latest，不会被旧版本覆盖。
为什么需要
GitHub 上标记为 prerelease 的
0.2.0对已经安装稳定版0.2.0的用户并不是更高版本，后续同版本构建也无法触发自动更新。此前本地和 CI 每次都会重新下载 Node.js，并且下载前已经删除旧 runtime；下载超时后，手动继续打包可能得到缺少 Node 的半成品。Windows 产物的递归搜索还会把内置rg.exe发布为独立资产。Reviewer Test Plan
如何验证
0.2.1）触发预发任务，应在元数据阶段被拒绝；0.2.1-rc.1应通过。*-setup.exe和对应更新签名。证据（修改前后）
修改前：预发可以复用稳定版本号，runtime 会在网络下载前被删除，Node 每次重新下载，递归收集可能发布内置可执行文件。
修改后：预发版本可安全升级，已校验 Node 归档可复用，runtime 仅在校验完成后开始替换，并能在替换中断后的下一次运行恢复；发布资产使用平台白名单，OSS feed 不会在稳定版发布后静默落后或覆盖更高版本。
测试平台
环境（可选）
macOS arm64，Node.js 22.22.0。针对性 ESLint、Desktop release helper、23 个发布/OSS 脚本测试、YAML 解析、diff 检查以及干净依赖下的
npm run build -- --cli-only均通过。真实无缓存 Node 下载仍复现了现有 120 秒网络超时；可执行回归已经覆盖缓存命中和组装失败时保留旧 runtime。风险与范围
prerelease时必须使用0.2.1-rc.1这类版本。关联 Issue
没有关联 Issue。