fix(desktop): close the round-1 review follow-ups on the arm64 leg

All six round-1 suggestions, plus two gaps found while re-checking the change independently: - Warn on stdout when --allow-missing-platform actually drops a key. A tolerant mirror run was byte-identical in the log to a complete one, so a feed published without linux-aarch64 was only discoverable by diffing it against the previous mirror -- and the OSS feed is the first updater endpoint, so arm64 clients would not fall through to GitHub either. - Accumulate repeated flags in parseArguments. The new option's two spellings disagreed: comma worked, repeated flags silently kept only the last value and then failed with an error naming a build leg. - List create-electron-bridge-manifest.mjs in the desktop_shell changed-files filter beside its sibling. test-release.js is the only test either feed script has and it runs in that job, so a PR touching only the bridge script skipped its own test and reported green. That was harmless while one Linux AppImage existed; pinning the selector to `_amd64` is what made the filter load-bearing. - desktop README: the Electron bridge publishes the *x64* Linux AppImage, which stopped being unambiguous once a release carries two. - Tests: pin the manifest_args expansion rather than only its pieces, pin that the publish step never tolerates a missing leg, pin that the filter lists both scripts, and pin both spellings of the multi-valued option. - test-release.js: reuse the runManifest helper for the trailing invocation it subsumes. Every new assertion was mutation-tested: dropping the expansion, making publish tolerant, unlisting the bridge script, reverting the flag accumulation and removing the warning each turn a suite red. Co-authored-by: Qwen-Coder <[email protected]>
yiliang114 · Sep 28, 2026 · Sep 27, 2026 · Sep 27, 2026 · Sep 27, 2026 · Sep 27, 2026
commit cfa6962d16b10a6d628c553bb27f5d37071d7eed
diff --git a/.github/scripts/create-desktop-update-manifest.mjs b/.github/scripts/create-desktop-update-manifest.mjs
@@ -69,7 +69,17 @@ fs.writeFileSync(options.output, `${JSON.stringify(manifest, null, 2)}\n`);
 
 function selectArtifact(assets, pattern, platform) {
   const matches = assets.filter((asset) => pattern.test(asset));
-  if (matches.length === 0 && allowMissingPlatforms.has(platform)) return null;
+  if (matches.length === 0 && allowMissingPlatforms.has(platform)) {
+    // stdout, not stderr: GitHub parses workflow commands from stdout only,
+    // and stderr has to stay reserved for the thrown error the tests match on.
+    // Without this a tolerant run is byte-identical in the log to a complete
+    // one, and the dropped key is only discoverable by diffing the published
+    // feed against the previous mirror.
+    console.log(
+      `::warning::no updater artifact for ${platform}; publishing the feed without it (--allow-missing-platform)`,
+    );
+    return null;
+  }
   if (matches.length !== 1) {
     throw new Error(
       `Expected one updater artifact for ${platform}, found ${matches.length}: ${matches.join(', ')}`,
@@ -90,7 +100,11 @@ function parseArguments(args) {
     const name = args[index]?.replace(/^--/, '');
     const value = args[index + 1];
     if (!name || value === undefined) throw new Error('Invalid arguments.');
-    values[name] = value;
+    // Accumulate repeats: a bash-array caller spells a multi-valued option as
+    // repeated flags, and plain assignment would keep only the last one — the
+    // dropped value then surfaces as a missing build leg during a release run.
+    values[name] =
+      values[name] === undefined ? value : `${values[name]},${value}`;
   }
   for (const required of ['assets', 'repository', 'tag', 'version', 'output']) {
     if (!values[required]) throw new Error(`Missing --${required}`);

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -2377,8 +2377,11 @@ jobs:
           if [[ "${GITHUB_EVENT_NAME}" == "pull_request" && -n "${PR_NUMBER}" ]]; then
             # `previous_filename` too: renaming a file out of the crate changes
             # it, and only the old path says so.
+            #
+            # Both feed scripts are listed because `scripts/test-release.js`
+            # is the only test either of them has, and it runs in this job.
             if files="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" --jq '.[] | .filename, (.previous_filename // empty)')"; then
-              if grep -Eq '^(packages/desktop/|\.github/scripts/create-desktop-update-manifest\.mjs|\.github/workflows/ci\.yml|\.github/workflows/desktop-release\.yml)' <<<"${files}"; then
+              if grep -Eq '^(packages/desktop/|\.github/scripts/create-desktop-update-manifest\.mjs|\.github/scripts/create-electron-bridge-manifest\.mjs|\.github/workflows/ci\.yml|\.github/workflows/desktop-release\.yml)' <<<"${files}"; then
                 changed=true
               else
                 changed=false

diff --git a/packages/desktop/README.md b/packages/desktop/README.md
@@ -68,6 +68,6 @@ cargo test --manifest-path src-tauri/Cargo.toml
 
 The `Desktop Release` workflow builds signed updater artifacts when `dry_run` is disabled. Published releases require the Tauri updater private key. macOS releases also require Apple signing and notarization credentials.
 
-The first stable Tauri release may set `electron_bridge=true` to publish the macOS ZIPs and DMGs, Windows NSIS installer, Linux AppImage, and their Electron `0.0.5` manifests. Leave the input disabled for later releases; the fixed `desktop-latest` release retains the bridge assets while `desktop-latest.json` advances independently.
+The first stable Tauri release may set `electron_bridge=true` to publish the macOS ZIPs and DMGs, Windows NSIS installer, x64 Linux AppImage, and their Electron `0.0.5` manifests. Leave the input disabled for later releases; the fixed `desktop-latest` release retains the bridge assets while `desktop-latest.json` advances independently.
 
 The macOS workflow accepts either the Tauri-era `APPLE_*` certificate and notarization secrets or the existing `MAC_CSC_*` and `APPLE_NOTARY_*` secrets. `TAURI_SIGNING_PRIVATE_KEY` must match the public key in `src-tauri/tauri.conf.json`.
diff --git a/packages/desktop/scripts/test-release.js b/packages/desktop/scripts/test-release.js
@@ -1459,43 +1459,61 @@ function testUpdateManifest(directory) {
     missingLeg.stderr,
     /Expected one updater artifact for linux-aarch64, found 0/,
   );
+  assert.doesNotMatch(
+    missingLeg.stdout,
+    /::warning::/,
+    'a run that refused to publish must not also report a tolerated platform',
+  );
   assert.match(
     runManifest('--allow-missing-platform', 'darwin-x86_64').stderr,
     /linux-aarch64, found 0/,
     'the escape hatch is keyed per platform, not a blanket opt-out',
   );
-  assert.equal(
-    runManifest('--allow-missing-platform', 'linux-aarch64').status,
-    0,
-  );
+  const tolerant = runManifest('--allow-missing-platform', 'linux-aarch64');
+  assert.equal(tolerant.status, 0, tolerant.stderr);
   assert.deepEqual(
     Object.keys(JSON.parse(fs.readFileSync(output, 'utf8')).platforms).sort(),
     ['darwin-aarch64', 'darwin-x86_64', 'linux-x86_64', 'windows-x86_64'],
   );
-  fs.writeFileSync(path.join(assets, artifacts[4]), artifacts[4]);
-  fs.writeFileSync(
-    path.join(assets, `${artifacts[4]}.sig`),
-    `signature:${artifacts[4]}\n`,
+  // A dropped key is otherwise invisible: the run exits 0 and its log is
+  // byte-identical to a complete one, so the omission is only discoverable by
+  // diffing the published feed against the previous mirror.
+  assert.match(
+    tolerant.stdout,
+    /::warning::no updater artifact for linux-aarch64/,
   );
 
-  fs.rmSync(path.join(assets, `${artifacts[3]}.sig`));
-  const failure = spawnSync(
-    process.execPath,
+  // Both spellings of a multi-valued option must mean the same thing: a
+  // bash-array caller writes repeated flags, and plain assignment in
+  // parseArguments would silently keep only the last one.
+  fs.rmSync(path.join(assets, artifacts[0]));
+  fs.rmSync(path.join(assets, `${artifacts[0]}.sig`));
+  for (const spelling of [
     [
-      manifestScript,
-      '--assets',
-      assets,
-      '--repository',
-      'QwenLM/qwen-code',
-      '--tag',
-      'desktop-v0.1.0',
-      '--version',
-      '0.1.0',
-      '--output',
-      output,
+      '--allow-missing-platform',
+      'linux-aarch64',
+      '--allow-missing-platform',
+      'darwin-aarch64',
     ],
-    { encoding: 'utf8' },
-  );
+    ['--allow-missing-platform', 'linux-aarch64,darwin-aarch64'],
+  ]) {
+    const both = runManifest(...spelling);
+    assert.equal(both.status, 0, both.stderr);
+    assert.deepEqual(
+      Object.keys(JSON.parse(fs.readFileSync(output, 'utf8')).platforms).sort(),
+      ['darwin-x86_64', 'linux-x86_64', 'windows-x86_64'],
+    );
+  }
+  for (const artifact of [artifacts[0], artifacts[4]]) {
+    fs.writeFileSync(path.join(assets, artifact), artifact);
+    fs.writeFileSync(
+      path.join(assets, `${artifact}.sig`),
+      `signature:${artifact}\n`,
+    );
+  }
+
+  fs.rmSync(path.join(assets, `${artifacts[3]}.sig`));
+  const failure = runManifest();
   assert.notEqual(failure.status, 0);
   assert.match(failure.stderr, /Missing updater signature/);
 }

diff --git a/scripts/tests/desktop-isolation.test.js b/scripts/tests/desktop-isolation.test.js
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-// Two agreements nothing else in the suite reads, both of which fail green.
+// Three agreements nothing else in the suite reads, all of which fail green.
 //
 // 1. `scripts/check-desktop-isolation.js` keeps a `nativePrefixes` list, and
 //    the root workspace manifests keep a `!packages/*` negation per native
@@ -23,6 +23,12 @@
 //    other and a directory that exists, or the job skips and reports success
 //    having compiled nothing.
 //
+// 3. That job's changed-files filter lists both updater-feed scripts.
+//    `packages/desktop/scripts/test-release.js` is the only test either of them
+//    has and it runs in this job, so a filter naming one script and not the
+//    other lets a PR that edits only the unlisted one skip its own test and
+//    still report green.
+//
 // The npm/pnpm mirror of the negation list is already pinned by
 // `package-scripts.test.js` ("mirrors the npm workspace boundaries in
 // pnpm-workspace.yaml"), so these tests read `package.json` only.
@@ -210,4 +216,16 @@ describe('desktop_shell CI job — the crate path agrees with itself', () => {
       `the filter alternative ${filterAlternative} has no trailing slash, so the changed-files filter also matches sibling paths that merely start with it`,
     ).toBe(true);
   });
+
+  it('lists both updater-feed scripts, so either one triggers the lane that tests them', () => {
+    for (const script of [
+      'create-desktop-update-manifest',
+      'create-electron-bridge-manifest',
+    ]) {
+      expect(
+        filterRun,
+        `the changed-files filter does not list ${script}.mjs, so a PR touching only that script skips 'Run desktop release tests' and reports green having tested nothing`,
+      ).toContain(`\\.github/scripts/${script}\\.mjs`);
+    }
+  });
 });
diff --git a/scripts/tests/desktop-oss-workflow.test.js b/scripts/tests/desktop-oss-workflow.test.js
@@ -343,6 +343,13 @@ describe('Desktop OSS mirror workflow', () => {
     expect(prepare).toContain(
       'manifest_args=(--allow-missing-platform linux-aarch64)',
     );
+    // Pin the effect, not only the pieces: the declaration and the `if` guard
+    // both survive a refactor that drops the expansion, and an unexpanded array
+    // is not an error under `set -u` — the mirror would then die with
+    // `found 0` on exactly the path this flag exists for.
+    expect(prepare).toMatch(
+      /create-desktop-update-manifest\.mjs[\s\S]*?"\$\{manifest_args\[@\]}"/,
+    );
     expect(prepare).toContain('sha256sum -- * > SHA256SUMS.txt');
 
     const upload = getWorkflowStep(
@@ -376,6 +383,16 @@ describe('Desktop OSS mirror workflow', () => {
 
   it('advances the OSS feed only for the current GitHub stable version', () => {
     const publish = getWorkflowJob(releaseWorkflow, 'publish');
+    // The other side of the strictness split. A fresh build must never be able
+    // to tolerate a missing leg: an operator unblocking a release by adding the
+    // flag here would drop the key from the primary feed while the arm64 asset
+    // still ships — #12806 again, with every test green and no step failing.
+    const generateManifest = getWorkflowStep(
+      publish,
+      'Generate checksums and updater manifest',
+    );
+    expect(generateManifest).toContain('create-desktop-update-manifest.mjs');
+    expect(generateManifest).not.toContain('--allow-missing-platform');
     const updateFeed = getWorkflowStep(publish, 'Update stable updater feed');
     expect(updateFeed).toContain('sort -V');
     expect(updateFeed).toContain(