From 3aa90434a07a657e1ba6af7dea99606c828347d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=86=E9=80=8A?= <72533078+UncertaintyDeterminesYou4ndMe@users.noreply.github.com> Date: Sat, 22 Aug 2026 13:39:26 +0800 Subject: [PATCH] fix(desktop): show the canonical Maka icon in the permission guide app.getFileIcon() was the wrong identity source: macOS reduces the path to its UTType and returns the generic application icon, never this app's own. Load the canonical 1024px assets/icon.png (shipped since the window-icon fix) through desktopAssetPath + nativeImage.createFromPath instead, resolved lazily at the card payload so non-darwin starts never pay the decode. This retires the whole native icon chain: both getFileIcon() calls, BUNDLE_ICON_OPTIONS, loadNativeBundleIcon() and its packaged-only gate, the drag-time native fallback, and the unbounded icon await in start(). Dev builds now show the icon too instead of a deliberate blank. The PNG is now a load-bearing packaged resource, so assertPackagedResources requires it on current builds, gated off for the legacy Windows upgrade baseline that predates it. Implements the follow-up agreed in #3455 review. Refs #3352. Generated-by: Claude Code --- apps/desktop/electron-builder.config.mjs | 9 ++-- .../permission-overlay-controller.test.ts | 23 +--------- .../src/main/permission-overlay/app-bundle.ts | 28 ----------- .../permission-overlay-main.ts | 46 +++++++++---------- scripts/verify-packaged-app.mjs | 6 +++ scripts/verify-packaged-app.test.mjs | 36 +++++++++++++++ scripts/verify-windows-x64.mjs | 1 + 7 files changed, 72 insertions(+), 77 deletions(-) diff --git a/apps/desktop/electron-builder.config.mjs b/apps/desktop/electron-builder.config.mjs index a22fcfbd1e..829a714fd7 100644 --- a/apps/desktop/electron-builder.config.mjs +++ b/apps/desktop/electron-builder.config.mjs @@ -52,10 +52,11 @@ export default { to: 'bundled-tools.json', }, { - // The app icon is read at runtime by the BrowserWindow `icon` option, and - // `files` above does not carry `assets/`. Electron reports the missing - // file as an empty image rather than an error, so without this the - // packaged app just draws no window icon. + // The app icon is read at runtime by the BrowserWindow `icon` option + // and by the permission-overlay card, and `files` above does not carry + // `assets/`. Electron reports the missing file as an empty image rather + // than an error, so without this the packaged app just draws no window + // icon; `assertPackagedResources` requires it on current builds. from: 'assets', to: 'assets', }, diff --git a/apps/desktop/src/main/__tests__/permission-overlay-controller.test.ts b/apps/desktop/src/main/__tests__/permission-overlay-controller.test.ts index aa43c203d1..36bf312f4e 100644 --- a/apps/desktop/src/main/__tests__/permission-overlay-controller.test.ts +++ b/apps/desktop/src/main/__tests__/permission-overlay-controller.test.ts @@ -20,11 +20,7 @@ import { type PermissionOverlayDeps, type PermissionOverlayWindowLike, } from '../permission-overlay/permission-overlay-controller.js'; -import { - BUNDLE_ICON_OPTIONS, - loadNativeBundleIcon, - resolveAppBundle, -} from '../permission-overlay/app-bundle.js'; +import { resolveAppBundle } from '../permission-overlay/app-bundle.js'; /** Deterministic timer wheel — no real time passes in these tests. */ function createClock() { @@ -307,23 +303,6 @@ describe('drag-to-grant permission overlay', () => { }); describe('app bundle resolution for the drag', () => { - it('never calls the native icon loader for an unpackaged app', async () => { - let calls = 0; - const icon = await loadNativeBundleIcon(false, async () => { - calls += 1; - return 'icon'; - }); - assert.equal(icon, null); - assert.equal(calls, 0, 'unpackaged development must not call app.getFileIcon()'); - assert.equal(await loadNativeBundleIcon(true, async () => 'icon'), 'icon'); - }); - - it('never requests the large icon size that kills packaged macOS builds', () => { - // 'large' hits a fatal NOTREACHED inside Chromium's IconLoader on - // macOS (SIGTRAP, not a catchable error) — see issue #3352. - assert.notEqual(BUNDLE_ICON_OPTIONS.size as string, 'large'); - }); - it('walks three levels up from the executable to the .app', () => { assert.deepEqual( resolveAppBundle({ diff --git a/apps/desktop/src/main/permission-overlay/app-bundle.ts b/apps/desktop/src/main/permission-overlay/app-bundle.ts index 01767a101d..1fd02d11cd 100644 --- a/apps/desktop/src/main/permission-overlay/app-bundle.ts +++ b/apps/desktop/src/main/permission-overlay/app-bundle.ts @@ -34,34 +34,6 @@ export interface ResolveAppBundleDeps { exists(path: string): boolean; } -/** - * The only size every platform can actually deliver. `'large'` is - * unsupported on macOS: Chromium's IconLoader hits a fatal NOTREACHED and - * the process dies with SIGTRAP before the promise settles — no JavaScript - * error is ever thrown, so the try/catch in `loadNativeBundleIcon` cannot - * save the app. Callers upscale the 32x32 result as needed. - */ -export const BUNDLE_ICON_OPTIONS = { size: 'normal' } as const; - -/** - * Reading a bundle icon is presentation-only. The original unpackaged npm - * Electron runtime could terminate natively while macOS resolved its bundle - * icon, before the returned promise settled. Keep native icon loading for - * packaged Maka.app builds and let the signed Maka Dev workflow use an empty - * drag image instead. - */ -export async function loadNativeBundleIcon( - isPackaged: boolean, - load: () => Promise, -): Promise { - if (!isPackaged) return null; - try { - return await load(); - } catch { - return null; - } -} - export function resolveAppBundle(deps: ResolveAppBundleDeps): AppBundleResult { const { executablePath, platform, exists } = deps; if (platform !== 'darwin') { diff --git a/apps/desktop/src/main/permission-overlay/permission-overlay-main.ts b/apps/desktop/src/main/permission-overlay/permission-overlay-main.ts index 9aa7701974..61948d0757 100644 --- a/apps/desktop/src/main/permission-overlay/permission-overlay-main.ts +++ b/apps/desktop/src/main/permission-overlay/permission-overlay-main.ts @@ -22,9 +22,10 @@ import { createRequire } from 'node:module'; import { existsSync } from 'node:fs'; import { join } from 'node:path'; import type { UiLocale } from '@maka/core/ui-locale'; +import { desktopAssetPath } from '../desktop-assets.js'; import { resolveOverlayAssetDir } from '../overlay-assets.js'; import { openSystemPermissionPane, requestPermissionAccess } from '../permissions-actions.js'; -import { BUNDLE_ICON_OPTIONS, loadNativeBundleIcon, resolveAppBundle } from './app-bundle.js'; +import { resolveAppBundle } from './app-bundle.js'; import { getPermissionOverlayCopy } from './permission-overlay-copy.js'; import { createPermissionOverlayController, @@ -64,7 +65,7 @@ export function createPermissionOverlayMain( let locale: UiLocale = 'en'; let iconDataUrl: string | null = null; const electron = requireElectron('electron') as Electron; - const { BrowserWindow, app, screen, systemPreferences } = electron; + const { BrowserWindow, app, nativeImage, screen, systemPreferences } = electron; function isGranted(id: DragGrantPermissionId): boolean { if (process.platform !== 'darwin') return false; @@ -74,15 +75,21 @@ export function createPermissionOverlayMain( return systemPreferences.getMediaAccessStatus('screen') === 'granted'; } - async function resolveAppIconDataUrl(bundlePath: string | null): Promise { - if (!bundlePath) return null; - const icon = await loadNativeBundleIcon(app.isPackaged, () => - app.getFileIcon(bundlePath, BUNDLE_ICON_OPTIONS), + function resolveAppIconDataUrl(): string | null { + // The canonical 1024px icon ships in assets/ — the same PNG + // electron-builder stamps onto the bundle. `app.getFileIcon()` is the + // wrong source for an identity: macOS reduces the path to its UTType + // and returns the generic application icon, never this app's own — and + // requesting it at size 'large' killed packaged builds outright with a + // fatal NOTREACHED inside Chromium's IconLoader (#3352). + const icon = nativeImage.createFromPath( + desktopAssetPath( + { isPackaged: app.isPackaged, resourcesPath: process.resourcesPath }, + 'assets', + 'icon.png', + ), ); - if (!icon || icon.isEmpty()) return null; - // nativeImage.createFromPath does not decode .icns reliably. Asking - // macOS for the bundle icon returns the same image Finder and the TCC - // list display for packaged Maka builds. + if (icon.isEmpty()) return null; return icon.resize({ width: 64, height: 64 }).toDataURL(); } @@ -110,6 +117,10 @@ export function createPermissionOverlayMain( exists: existsSync, }); const bundlePath = bundle.ok ? bundle.bundlePath : null; + // Resolved here, at the only consumer, so a non-darwin start() never + // pays the PNG decode; a null result (asset missing) retries on the + // next card rather than caching the failure. + iconDataUrl ??= resolveAppIconDataUrl(); return { permission: id, appName: app.getName(), @@ -187,12 +198,6 @@ export function createPermissionOverlayMain( } catch (error) { console.warn('[permission-overlay] locale lookup failed, keeping', locale, error); } - const bundle = resolveAppBundle({ - executablePath: app.getPath('exe'), - platform: process.platform, - exists: existsSync, - }); - iconDataUrl = await resolveAppIconDataUrl(bundle.ok ? bundle.bundlePath : null); return controller.start(id); }, }; @@ -267,18 +272,13 @@ function attachCardGestures(win: import('electron').BrowserWindow): void { payload && typeof payload === 'object' && 'iconDataUrl' in payload ? (payload as { iconDataUrl?: unknown }).iconDataUrl : null; + // The file drag still works without a decorative drag image, so a + // failed decode degrades to an empty icon rather than a native read. let icon = nativeImage.createEmpty(); if (typeof iconDataUrl === 'string' && iconDataUrl.startsWith('data:image/')) { const fromRenderer = nativeImage.createFromDataURL(iconDataUrl); if (!fromRenderer.isEmpty()) icon = fromRenderer; } - if (icon.isEmpty()) { - const fallback = await loadNativeBundleIcon(app.isPackaged, () => - app.getFileIcon(resolved.bundlePath, BUNDLE_ICON_OPTIONS), - ); - if (fallback && !fallback.isEmpty()) icon = fallback.resize({ width: 64, height: 64 }); - // The file drag still works without a decorative drag image. - } if (!win.isDestroyed()) win.webContents.startDrag({ file: resolved.bundlePath, icon }); }); diff --git a/scripts/verify-packaged-app.mjs b/scripts/verify-packaged-app.mjs index 1ef233394b..88a7fa7e4d 100644 --- a/scripts/verify-packaged-app.mjs +++ b/scripts/verify-packaged-app.mjs @@ -714,6 +714,11 @@ export async function assertPackagedResources( // build, which predates the disclaimer being packaged. Requiring it there // would fail a release that was correct when it shipped. requireDisclaimer = true, + // Same shape as the disclaimer: the canonical icon began shipping as an + // extra resource with the window-icon fix, and the permission overlay + // reads it at runtime, so current builds must carry it — but a + // previously released baseline predates it. + requireCanonicalIcon = true, } = {}, ) { if (bundledGitContract !== 'forbidden' && bundledGitContract !== 'legacy-required') { @@ -732,6 +737,7 @@ export async function assertPackagedResources( join('licenses', 'git', 'NOTICE.txt'), ] : []), + ...(requireCanonicalIcon ? [join('assets', 'icon.png')] : []), join('workers', 'filesystem-worker.js'), join('licenses', 'maka', 'LICENSE'), join('licenses', 'maka', 'NOTICE'), diff --git a/scripts/verify-packaged-app.test.mjs b/scripts/verify-packaged-app.test.mjs index f529a50a93..ef7ec3daaf 100644 --- a/scripts/verify-packaged-app.test.mjs +++ b/scripts/verify-packaged-app.test.mjs @@ -38,6 +38,7 @@ test('legacy packaged resources require the historical bundled Git contract', as forbidPath: async (path) => forbidden.push(path), requireWindowsSandbox: false, bundledGitContract: 'legacy-required', + requireCanonicalIcon: false, }); for (const path of [ @@ -304,3 +305,38 @@ describe('assertPackagedDependencyClosure', () => { ); }); }); + +// The resource list is contract, not implementation: the permission overlay +// reads `assets/icon.png` at runtime, so a current build that drops it ships +// a regression the app cannot report. The check is driven through the +// injectable `requirePath`, so it needs no packaging and no platform. +describe('assertPackagedResources', () => { + const resources = join('fake', 'resources'); + const iconPath = join(resources, 'assets', 'icon.png'); + const requirePathMissing = (absent) => async (path) => { + if (path === absent) throw new Error(`MISSING ${path}`); + }; + const forbidPath = async () => {}; + + test('a current build must carry the canonical icon', async () => { + await assert.rejects( + () => + assertPackagedResources(resources, { + requirePath: requirePathMissing(iconPath), + forbidPath, + requireWindowsSandbox: false, + }), + /MISSING .*icon\.png/, + ); + }); + + test('a legacy baseline predating the packaged icon is not required to carry it', async () => { + await assertPackagedResources(resources, { + requirePath: requirePathMissing(iconPath), + forbidPath, + requireWindowsSandbox: false, + requireDisclaimer: false, + requireCanonicalIcon: false, + }); + }); +}); diff --git a/scripts/verify-windows-x64.mjs b/scripts/verify-windows-x64.mjs index 5ba87c7432..3cfe97bd56 100644 --- a/scripts/verify-windows-x64.mjs +++ b/scripts/verify-windows-x64.mjs @@ -110,6 +110,7 @@ export async function verifyPackagedWindowsApp( requireWindowsSandbox: requiresCurrentContract, requireDisclaimer: requiresCurrentContract, bundledGitContract: requiresCurrentContract ? 'forbidden' : 'legacy-required', + requireCanonicalIcon: requiresCurrentContract, }); if (requiresCurrentContract) await assertPackagedDependencyClosure(resources); else await requirePath(join(resources, 'git', 'cmd', 'git.exe'));