fix(release): harden package verification and release manifests (#80 follow-up)

.github/workflows/release.yml

@@ -12,6 +12,9 @@ jobs:
     # Only build assets on prereleased to avoid duplicate builds
     if: github.event.action == 'prereleased'
     strategy:
+      # Let the other arches finish, so one run shows every failure. The run still concludes as
+      # failed, and upload-manifests only promotes from a successful one.
+      fail-fast: false
       matrix:
         # Each arch builds on its own native runner: npm and Prisma only install the prebuilt
         # binaries of the host arch, and electron-builder doesn't fetch those of another arch.
@@ -20,18 +23,23 @@ jobs:
           - os: macos-15-intel
             platform: mac
             arch: x64
+            manifest: latest-mac.yml
           - os: macos-latest
             platform: mac
             arch: arm64
+            manifest: latest-mac.yml
           - os: windows-latest
             platform: win
             arch: x64
+            manifest: latest.yml
           - os: ubuntu-latest
             platform: linux
             arch: x64
+            manifest: latest-linux.yml
           - os: ubuntu-24.04-arm
             platform: linux
             arch: arm64
+            manifest: latest-linux-arm64.yml
 
     runs-on: ${{ matrix.os }}
 
@@ -93,6 +101,18 @@ jobs:
           APPLE_APP_SPECIFIC_PASSWORD: ${{ secrets.APPLE_APP_SPECIFIC_PASSWORD }}
           APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }}
 
+      # The exact file rather than a glob, which other yml files in release/ would satisfy. A missing
+      # manifest fails the run here, before this job publishes its assets, not at promotion to stable.
+      # A re-run rebuilds the installers, so its manifest has to replace the one describing the old ones.
+      - name: Save update manifest as artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: update-manifests-${{ matrix.platform }}-${{ matrix.arch }}
+          path: release/${{ matrix.manifest }}
+          if-no-files-found: error
+          retention-days: 30
+          overwrite: true
+
       - name: Upload release assets (installers only)
         uses: softprops/action-gh-release@v2
         with:
@@ -107,13 +127,6 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
-      - name: Save update manifests as artifacts
-        uses: actions/upload-artifact@v4
-        with:
-          name: update-manifests-${{ matrix.platform }}-${{ matrix.arch }}
-          path: release/*.yml
-          retention-days: 30
-
   # Upload auto-update manifests only when release is promoted to stable
   upload-manifests:
     if: github.event.action == 'released'
@@ -150,14 +163,6 @@ jobs:
           echo "Downloaded manifests:"
           find manifests -type f -name "*.yml" | sort
 
-      - name: Filter out builder-debug.yml
-        run: |
-          # Remove builder-debug.yml from manifests directory (exists in all platform artifacts)
-          find manifests -name "builder-debug.yml" -delete
-
-          echo "Remaining manifests:"
-          find manifests -type f -name "*.yml" | sort
-
       # electron-updater reads one latest-mac.yml and picks the file for its arch from it.
       - name: Merge macOS manifests
         run: |

scripts/__tests__/package-load-check.test.mjs

@@ -0,0 +1,122 @@
+import { spawnSync } from 'node:child_process';
+import { mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+
+const SCRIPT = fileURLToPath(new URL('../package-load-check.cjs', import.meta.url));
+
+/** A canvas whose every pixel reads back black, which satisfies the checks that count dark pixels. */
+const FAKE_CANVAS = `
+module.exports = {
+  createCanvas: () => ({
+    encode: async () => Buffer.alloc(8),
+    getContext: () => ({
+      fillRect() {},
+      drawImage() {},
+      getImageData: (_x, _y, width, height) => ({ data: new Uint8ClampedArray(width * height * 4) }),
+    }),
+  }),
+  loadImage: async () => ({ width: 20, height: 10 }),
+};
+`;
+
+/** As many pages as the sample PDF has; the fake canvas decides what they look like. */
+const FAKE_PDF_RENDERER = `
+exports.renderPdfPages = async () => Array.from({ length: 5 }, () => ({ content: Buffer.alloc(0) }));
+`;
+
+/** A Prisma client that fails where the test's environment tells it to, as a packaged one might. */
+const FAKE_PRISMA = `
+const fs = require('node:fs');
+const { FAIL_QUERY, FAIL_DISCONNECT, FAIL_REMOVE } = process.env;
+
+if (FAIL_REMOVE) {
+  fs.rmSync = (_dir, options) => {
+    throw new Error(options.maxRetries > 0 ? 'EBUSY after retrying' : 'EBUSY without retrying');
+  };
+}
+
+module.exports.PrismaClient = class {
+  async $queryRaw() {
+    if (FAIL_QUERY) throw new Error('engine not found');
+  }
+  async $disconnect() {
+    if (FAIL_DISCONNECT) throw new Error('disconnect failed');
+  }
+};
+`;
+
+describe('package-load-check', () => {
+  let root;
+  let tempDir;
+
+  /** Runs the script against a fake package, keeping its temp files in a directory of their own. */
+  const runCheck = (failures = {}) =>
+    spawnSync(process.execPath, [SCRIPT, path.join(root, 'resources')], {
+      encoding: 'utf8',
+      env: { ...process.env, TMPDIR: tempDir, TEMP: tempDir, TMP: tempDir, ...failures },
+    });
+
+  const putModule = (name, source) => {
+    const dir = path.join(root, 'resources', 'app.asar', 'node_modules', name);
+    mkdirSync(dir, { recursive: true });
+    writeFileSync(path.join(dir, 'index.js'), source);
+  };
+
+  beforeEach(() => {
+    root = mkdtempSync(path.join(tmpdir(), 'package-load-check-'));
+    tempDir = path.join(root, 'tmp');
+    mkdirSync(tempDir);
+    putModule('@napi-rs/canvas', FAKE_CANVAS);
+    putModule('pdf-to-png-converter', '');
+    putModule('@prisma/client', FAKE_PRISMA);
+    const rendererDir = path.join(root, 'resources', 'app.asar', 'dist', 'main');
+    mkdirSync(rendererDir, { recursive: true });
+    writeFileSync(path.join(rendererDir, 'pdf-page-renderer.js'), FAKE_PDF_RENDERER);
+  });
+
+  afterEach(() => {
+    rmSync(root, { recursive: true, force: true });
+  });
+
+  it('passes and removes its temp directory when everything loads', () => {
+    const result = runCheck();
+
+    expect(result.stdout).toContain('load ok    Prisma query engine');
+    expect(result.status).toBe(0);
+    expect(readdirSync(tempDir)).toEqual([]);
+  });
+
+  it('fails when a native module does not work', () => {
+    const result = runCheck({ FAIL_QUERY: '1' });
+
+    expect(result.stderr).toContain('load FAIL  Prisma query engine: engine not found');
+    expect(result.status).toBe(1);
+  });
+
+  it('only warns when its temp directories cannot be removed, after retrying', () => {
+    const result = runCheck({ FAIL_REMOVE: '1' });
+
+    expect(result.stdout).toContain('load ok    Prisma query engine');
+    expect(result.stderr).toMatch(/warning: could not remove .*: EBUSY after retrying/);
+    expect(result.stderr).not.toContain('EBUSY without retrying');
+    expect(result.status).toBe(0);
+  });
+
+  it('only warns when Prisma fails to disconnect, and still removes the temp directory', () => {
+    const result = runCheck({ FAIL_DISCONNECT: '1' });
+
+    expect(result.stderr).toContain('warning: could not disconnect Prisma: disconnect failed');
+    expect(result.status).toBe(0);
+    expect(readdirSync(tempDir)).toEqual([]);
+  });
+
+  it('keeps the original error when cleanup fails too', () => {
+    const result = runCheck({ FAIL_QUERY: '1', FAIL_DISCONNECT: '1', FAIL_REMOVE: '1' });
+
+    expect(result.stderr).toContain('load FAIL  Prisma query engine: engine not found');
+    expect(result.status).toBe(1);
+  });
+});

scripts/__tests__/verify-package-arch.test.mjs

@@ -1,8 +1,12 @@
-import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { spawnSync } from 'node:child_process';
+import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import path from 'node:path';
+import { fileURLToPath } from 'node:url';
 import { afterEach, beforeEach, describe, expect, it } from 'vitest';
-import { parseBinaryHeader, resolvePackageLayout, verifyBinaries } from '../verify-package-arch.mjs';
+import { loadCheckProblems, parseBinaryHeader, resolvePackageLayout, verifyBinaries } from '../verify-package-arch.mjs';
+
+const SCRIPT = fileURLToPath(new URL('../verify-package-arch.mjs', import.meta.url));
 
 const CPU_TYPE = { x64: 0x01000007, arm64: 0x0100000c };
 
@@ -83,6 +87,56 @@ describe('resolvePackageLayout', () => {
   });
 });
 
+describe('loadCheckProblems', () => {
+  it('accepts a clean exit', () => {
+    expect(loadCheckProblems({ status: 0, signal: null })).toEqual([]);
+  });
+
+  it('reports the exit code of a failed check', () => {
+    expect(loadCheckProblems({ status: 1, signal: null })).toEqual(['Load check failed with exit code 1']);
+  });
+
+  it('names the signal when the check was killed, as there is no exit code then', () => {
+    expect(loadCheckProblems({ status: null, signal: 'SIGSEGV' })).toEqual(['Load check was killed by signal SIGSEGV']);
+  });
+
+  it('prefers the spawn error, which explains a timeout better than the signal it sends', () => {
+    const error = new Error('spawnSync ETIMEDOUT');
+
+    expect(loadCheckProblems({ error, status: null, signal: 'SIGTERM' })).toEqual([
+      'Load check could not run: spawnSync ETIMEDOUT',
+    ]);
+  });
+});
+
+describe('command line entry', () => {
+  // An unknown platform makes main() fail right away, so a non-zero exit proves that it ran.
+  const run = (script) => spawnSync(process.execPath, [script, '--platform', 'nope'], { encoding: 'utf8' });
+
+  it('runs when started directly', () => {
+    const result = run(SCRIPT);
+
+    expect(result.status).toBe(1);
+    expect(result.stderr).toContain('Unknown --platform "nope"');
+  });
+
+  // Creating a symlink needs extra privileges on Windows.
+  it.skipIf(process.platform === 'win32')('runs when started through a symlink, rather than passing silently', () => {
+    const dir = mkdtempSync(path.join(tmpdir(), 'verify-package-arch-link-'));
+    try {
+      const link = path.join(dir, 'verify.mjs');
+      symlinkSync(SCRIPT, link);
+
+      const result = run(link);
+
+      expect(result.status).toBe(1);
+      expect(result.stderr).toContain('Unknown --platform "nope"');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+});
+
 describe('verifyBinaries', () => {
   let resourcesDir;
   let executable;

scripts/package-load-check.cjs

@@ -17,6 +17,15 @@ const appDir = path.join(path.resolve(process.argv[2]), 'app.asar');
 const appModules = path.join(appDir, 'node_modules');
 const loadFromApp = (name) => require(path.join(appModules, name));
 
+/** Cleanup is not what's being checked: a failure here must neither fail the check nor mask its error. */
+async function cleanUp(description, action) {
+  try {
+    await action();
+  } catch (error) {
+    console.warn(`  warning: could not ${description}: ${error.message}`);
+  }
+}
+
 async function checkCanvas() {
   const canvasModule = loadFromApp('@napi-rs/canvas');
   const canvas = canvasModule.createCanvas(20, 10);
@@ -77,8 +86,11 @@ async function checkPrisma() {
     await client.$queryRaw`SELECT 1`;
     return loadedPrismaEngine();
   } finally {
-    await client.$disconnect();
-    fs.rmSync(tempDir, { recursive: true, force: true });
+    await cleanUp('disconnect Prisma', () => client.$disconnect());
+    // Windows can hold the db file for a moment after the engine disconnects.
+    await cleanUp(`remove ${tempDir}`, () =>
+      fs.rmSync(tempDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }),
+    );
   }
 }
 
@@ -107,7 +119,9 @@ async function main() {
   }
 
   process.chdir(os.tmpdir());
-  fs.rmSync(emptyCwd, { recursive: true, force: true });
+  await cleanUp(`remove ${emptyCwd}`, () =>
+    fs.rmSync(emptyCwd, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }),
+  );
   process.exit(failed ? 1 : 0);
 }
 

scripts/verify-package-arch.mjs

@@ -19,7 +19,7 @@
  */
 
 import { spawnSync } from 'node:child_process';
-import { closeSync, existsSync, openSync, readFileSync, readSync, readdirSync } from 'node:fs';
+import { closeSync, existsSync, openSync, readFileSync, readSync, readdirSync, realpathSync } from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { parseArgs } from 'node:util';
@@ -194,6 +194,14 @@ export function verifyBinaries({ resourcesDir, executable, platform, arch }) {
   return { binaries, problems };
 }
 
+/** @returns {string[]} problems found in the spawnSync result of the load check */
+export function loadCheckProblems({ error, status, signal }) {
+  if (error) return [`Load check could not run: ${error.message}`];
+  if (signal) return [`Load check was killed by signal ${signal}`];
+  if (status !== 0) return [`Load check failed with exit code ${status}`];
+  return [];
+}
+
 /** Loads the native modules in the packaged Electron. @returns {string[]} problems */
 function runLoadCheck({ resourcesDir, executable }) {
   const result = spawnSync(executable, [LOAD_CHECK_SCRIPT, resourcesDir], {
@@ -204,9 +212,7 @@ function runLoadCheck({ resourcesDir, executable }) {
   if (result.stdout) process.stdout.write(result.stdout);
   if (result.stderr) process.stderr.write(result.stderr);
 
-  if (result.error) return [`Load check could not run: ${result.error.message}`];
-  if (result.status !== 0) return [`Load check failed with exit code ${result.status}`];
-  return [];
+  return loadCheckProblems(result);
 }
 
 function main() {
@@ -258,7 +264,16 @@ function main() {
   console.log(`\nPackage ${platform}-${arch} is OK.`);
 }
 
-if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {
+/**
+ * Node resolves symlinks in this module's URL but not in argv[1], so comparing them directly would
+ * skip main() when started through a symlink, and the gate would pass with exit code 0.
+ */
+function isEntryPoint() {
+  const entry = process.argv[1];
+  return Boolean(entry) && existsSync(entry) && realpathSync(entry) === realpathSync(fileURLToPath(import.meta.url));
+}
+
+if (isEntryPoint()) {
   try {
     main();
   } catch (error) {