fix(forge): skip exe-icon-extractor rebuild on all platforms

[nerdCopter]

AI Generated pull-request

Summary

Two separate issues both caused by the `windows-2025` runner image being updated ~2026-06-16 to install Visual Studio 2025 (version 18) where VS 2022 (version 17) was previously installed:

Fix 1 — `forge.config.js`: skip `@bitdisaster/exe-icon-extractor` rebuild unconditionally

• Already excluded on Linux/macOS; extended to all platforms.
• On updated `windows-2025`, its source fails MSVC C2664 (string literals as `char*`). No upstream fix — versions 1.0.9 and 1.0.10 both fail.
• Safe to skip everywhere: `forge.config.js` always provides an explicit `icon:` path so icon-extraction is never invoked.
• Track upstream: https://github.com/bitdisaster/exe-icon-extractor

Fix 2 — `build.yml`: use `windows-2022` runner instead of `windows-2025`

• Updated `windows-2025` image replaced VS 2022 (v17) with VS 2025 (v18). No published version of `node-gyp` or `@electron/node-gyp` maps major version 18 to a year, causing VS detection to fail with `ERR_CHILD_PROCESS_STDIO_MAXBUFFER` — breaking `@serialport/bindings-cpp` rebuild during `electron-forge make`.
• `windows-2022` provides VS 2022 (v17) which all existing node-gyp versions recognize.
• Long-term fix (tracked separately): upgrade to `@electron-forge@8` + `node-gyp@12` which will gain VS 2025 support upstream, then revert runner to `windows-2025`.

Test plan

• Windows CI passes (`build (windows-2022, windows-x64, x64)` and `ia32`)
• Linux and macOS builds unaffected
• MSI and ZIP artifacts produced

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 803f07ad-fa97-4740-bd99-ed2dcdfcbcfe

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

In forge.config.js, the rebuildConfig.ignoreModules array is changed to unconditionally include @bitdisaster/exe-icon-extractor on all platforms, replacing a conditional that previously skipped it only on non-Windows. The surrounding comments are updated to note the module fails to rebuild on all platforms.

Changes

Forge rebuild config update

Layer / File(s)	Summary
Unconditional ignoreModules for exe-icon-extractor forge.config.js	rebuildConfig.ignoreModules now always excludes @bitdisaster/exe-icon-extractor regardless of process.platform; comments updated to reflect all-platform scope.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: unconditionally skipping the exe-icon-extractor module rebuild across all platforms, which is the core fix for the Windows CI build failures.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

forge.config.js (1)

69-72: 💤 Low value

Consider adding a tracking reference for future upstream fixes.

The comments clearly explain why this module must be ignored, but there's no mechanism to track when this workaround can be removed. Consider adding a reference to help future maintainers:

• A TODO comment with a link to track upstream fixes
• A reference to the upstream repository or issue
• A note about the package versions tested (1.0.9, 1.0.10)

Example:

// exe-icon-extractor is an optionalDependency of electron-wix-msi (maker-wix).
// Rebuilding it fails on Linux/macOS (node-gyp) and on Windows with the
// windows-2025 runner (MSVC C2664: string literals passed as char* rejected).
// It provides icon extraction only; maker-wix degrades gracefully without it.
// TODO: Monitor https://github.com/bitdisaster/exe-icon-extractor for fixes.
// Tested versions 1.0.9 and 1.0.10 both fail.

This is optional but improves long-term maintainability.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@forge.config.js` around lines 69 - 72, The comment block explaining the
exe-icon-extractor ignoring and the failures related to node-gyp on Linux/macOS
and MSVC C2664 on windows-2025 lacks any tracking mechanism for when this
workaround can be removed. Extend the existing comment block by adding a TODO
comment that references the upstream repository or issue tracker for
exe-icon-extractor, and include a note about the specific versions that were
tested (1.0.9 and 1.0.10) to help future maintainers identify when the fix can
be revisited.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@forge.config.js`:
- Around line 69-72: The comment block explaining the exe-icon-extractor
ignoring and the failures related to node-gyp on Linux/macOS and MSVC C2664 on
windows-2025 lacks any tracking mechanism for when this workaround can be
removed. Extend the existing comment block by adding a TODO comment that
references the upstream repository or issue tracker for exe-icon-extractor, and
include a note about the specific versions that were tested (1.0.9 and 1.0.10)
to help future maintainers identify when the fix can be revisited.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9a5d76f6-88f6-400c-a216-7339b669798d

📥 Commits

Reviewing files that changed from the base of the PR and between 8d56a80 and 77b65dd.

📒 Files selected for processing (1)

• forge.config.js

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Codacy Static Code Analysis

🔇 Additional comments (1)

forge.config.js (1)

69-74: Implementation is correct and complete.

The ignoreModules array is properly structured with both modules present:

• @bitdisaster/exe-icon-extractor added unconditionally (fails on all platforms)
• usb conditional preserved for Linux-only exclusion (with correct spread operator syntax)

Comments accurately describe the respective compilation issues and graceful degradation path.

[nerdCopter]

@coderabbitai, seemingly since this PR merge, at least Linux, the installable .deb no linger has an icon associated for the actual Application window (titlebar left-side has no custom icon).

make an issue ticket with your analysis/recommendations. If this is unsolvable using al the stable dependencies, then note such as a temporary blocker until newer versions released. #610 may still be a factor. or update the ticket if applicable.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== forge.config.js ==="
cat forge.config.js 2>/dev/null || echo "NOT FOUND"

echo ""
echo "=== package.json (icon references) ==="
cat package.json 2>/dev/null | python3 -c "import sys,json; d=json.load(sys.stdin); print(json.dumps(d.get('config',{}), indent=2))" 2>/dev/null || cat package.json 2>/dev/null | grep -i icon || echo "no icon refs"

echo ""
echo "=== Issue `#610` ==="
gh issue view 610 --repo emuflight/EmuConfigurator 2>/dev/null || echo "Could not read issue 610"

echo ""
echo "=== PR `#609` diff (forge.config.js) ==="
gh pr diff 609 --repo emuflight/EmuConfigurator 2>/dev/null | head -200

echo ""
echo "=== Search for icon references in forge.config.js and related files ==="
rg -n -i "icon" forge.config.js 2>/dev/null || echo "no results"

echo ""
echo "=== Find any icon files in the repo ==="
fd -e png -e ico -e icns . --exclude node_modules | head -30

Length of output: 13986

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Git log for forge.config.js (last 10 commits) ==="
git log --oneline -10 -- forge.config.js

echo ""
echo "=== Find main process entry point ==="
fd -e js -e ts main --exclude node_modules | head -20
cat main.js 2>/dev/null | grep -i "icon\|BrowserWindow" | head -30 || echo "main.js not found"

echo ""
echo "=== Search for BrowserWindow icon in source ==="
rg -n "BrowserWindow\|setIcon\|icon" src/ --type js -l 2>/dev/null | head -10

echo ""
echo "=== Check if emu_icon_128.png only exists or also other sizes ==="
ls -la assets/linux/icon/ 2>/dev/null

echo ""
echo "=== Check issue `#610` for sub-issues ==="
gh issue view 610 --repo emuflight/EmuConfigurator --json number,title,body,comments 2>/dev/null | python3 -c "
import sys, json
d = json.load(sys.stdin)
print('Number:', d['number'])
print('Title:', d['title'])
print('Comments count:', len(d.get('comments', [])))
for c in d.get('comments', []):
    print('  Comment by', c.get('author', {}).get('login'), ':', c.get('body', '')[:200])
"

echo ""
echo "=== Recent commits on current branch to identify any other changes around PR `#609` ==="
git log --oneline -20

echo ""
echo "=== Check if there's a desktop entry or install script referencing icons ==="
fd -e desktop -e sh -e yml . --exclude node_modules | xargs grep -l "icon" 2>/dev/null | head -10

Length of output: 3600

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

gh issue create \
  --repo emuflight/EmuConfigurator \
  --title "bug(linux): app window titlebar icon missing in installed .deb — asar packaging blocks BrowserWindow from reading icon at runtime" \
  --assignee nerdCopter \
  --body "## Summary

The application window titlebar icon is missing on Linux when running the installed \`.deb\` package. The window manager shows a blank/default icon instead of the EmuFlight icon in the titlebar (top-left).

Reported in: https://github.com/emuflight/EmuConfigurator/pull/609#issuecomment-HEAD  
Noticed around the merge of PR `#609`, but the direct cause is in the \`asar\` packaging path, not in the \`rebuildConfig.ignoreModules\` change introduced by that PR.

---

## Root Cause

**File:** \`main.js\`  
**Function:** \`getWindowIconPath()\`

\`\`\`js
// main.js (simplified)
function getWindowIconPath() {
  const iconCandidatesByPlatform = {
    linux: [
      path.join(__dirname, 'assets', 'linux', 'icon', 'emu_icon_128.png'),
      path.join(__dirname, 'src', 'images', 'emu_icon_128.png'),
      path.join(__dirname, 'dist', 'images', 'emu_icon_128.png'),
    ],
    // ...
  };
  const candidates = iconCandidatesByPlatform[process.platform] || [];
  for (const iconPath of candidates) {
    if (fs.existsSync(iconPath)) {
      return iconPath;      // ← returns an asar virtual path
    }
  }
}

const windowIconPath = getWindowIconPath();
const win = new BrowserWindow({ icon: windowIconPath, ... });
\`\`\`

**What goes wrong in the packaged \`.deb\`:**

1. \`forge.config.js\` sets \`packagerConfig.asar: true\` → the entire app (including \`assets/linux/icon/emu_icon_128.png\`) is packed into \`resources/app.asar\`.
2. Inside a packaged app, \`__dirname\` in \`main.js\` resolves to a virtual asar path (e.g. \`.../resources/app.asar/\`).
3. Electron patches Node's \`fs\` module so that \`fs.existsSync()\` returns \`true\` for paths inside the asar — so \`getWindowIconPath()\` successfully returns a path.
4. **However**, \`new BrowserWindow({ icon: '<asar-virtual-path>' })\` silently fails to set the window icon. The underlying OS window manager (e.g. X11/Wayland) needs a **real filesystem path** to a PNG file; it cannot read from inside an asar archive.
5. Result: no titlebar/window icon at runtime.

The \`maker-deb\` \`icon\` option and \`packagerConfig.icon\` (both pointing to \`assets/linux/icon/emu_icon_128.png\`) correctly affect the launcher/taskbar/file-manager icon at *install time* (those are extracted to the real filesystem by dpkg). They do **not** affect the runtime \`BrowserWindow\` titlebar icon.

---

## Relationship to PR `#609` and Issue `#610`

- **PR `#609`** changed only \`rebuildConfig.ignoreModules\` (extending \`exe-icon-extractor\` exclusion to all platforms) and the CI runner version. This change has **no direct effect** on the Linux window icon. The correlation is circumstantial — the issue was likely present before and is being noticed now.
- **Issue `#610`** (upgrade to \`@electron-forge v8\` + \`windows-2025\` runner) is **unrelated** to this icon bug. Keep tracking `#610` separately.

---

## Proposed Fixes (in order of preference)

### Option A — \`asarUnpack\` the icon + use \`process.resourcesPath\` in \`main.js\` (recommended)

**\`forge.config.js\`** — add \`asarUnpack\` glob:
\`\`\`js
packagerConfig: {
  asar: true,
  asarUnpack: ['assets/linux/icon/emu_icon_128.png'],
  // ... rest unchanged
},
\`\`\`

**\`main.js\`** — update Linux candidate to use the unpacked path:
\`\`\`js
// packaged: resources/app.asar.unpacked/assets/linux/icon/emu_icon_128.png
// dev:      __dirname/assets/linux/icon/emu_icon_128.png (fallback)
linux: [
  path.join(process.resourcesPath, 'app.asar.unpacked', 'assets', 'linux', 'icon', 'emu_icon_128.png'),
  path.join(__dirname, 'assets', 'linux', 'icon', 'emu_icon_128.png'), // dev fallback
  path.join(__dirname, 'src', 'images', 'emu_icon_128.png'),
  path.join(__dirname, 'dist', 'images', 'emu_icon_128.png'),
],
\`\`\`

### Option B — Load via \`nativeImage\` (asar-compatible)

Electron's \`nativeImage.createFromPath()\` *does* support reading from inside asars on Linux. Change \`main.js\` to:
\`\`\`js
const { nativeImage } = require('electron');

// Inside getWindowIconPath() or before creating BrowserWindow:
const iconPath = path.join(__dirname, 'assets', 'linux', 'icon', 'emu_icon_128.png');
const icon = nativeImage.createFromPath(iconPath);
const win = new BrowserWindow({ icon: icon.isEmpty() ? undefined : icon, ... });
\`\`\`
No forge.config.js change required. Works for dev and packaged.

### Option C — Copy icon to \`process.resourcesPath\` at build time

Less preferred; adds build complexity.

---

## Temporary status

This is a functional bug (no icon in titlebar). It does **not** block installation or usage. Issue `#610` (forge v8 upgrade) does not fix this; it must be addressed independently.

---

## Acceptance Criteria

- [ ] App window titlebar shows the EmuFlight icon on Linux when launched from the installed \`.deb\`
- [ ] Dev mode (\`yarn dev\`) continues to show the icon (fallback path still works)
- [ ] No regression on macOS or Windows window icons
- [ ] CI Linux build still produces a valid \`.deb\`
"

Length of output: 5430

---

---

@nerdCopter: Issue created → #614: #614

Here's a summary of the findings:

Root cause (not PR #609 directly):
main.js → getWindowIconPath() uses fs.existsSync() to locate the icon and passes the result to new BrowserWindow({ icon: ... }). With packagerConfig.asar: true, the icon lives inside app.asar. Electron patches fs so existsSync returns true for asar-internal paths — but BrowserWindow's icon option requires a real filesystem path; the OS window manager can't read from inside an asar archive, so the titlebar icon silently disappears. The .deb launcher/taskbar icon (set via maker-deb options) is unaffected since dpkg extracts that to the real filesystem at install time.

PR #609's rebuildConfig.ignoreModules change has no bearing on this — exe-icon-extractor was already excluded on Linux before that PR. The timing is coincidental.

Two recommended fixes are documented in the issue:

• Option A (recommended): Add asarUnpack: ['assets/linux/icon/emu_icon_128.png'] to forge.config.js + update the Linux candidate path in main.js to use process.resourcesPath + app.asar.unpacked/....
• Option B (simpler): Replace the fs.existsSync path approach with nativeImage.createFromPath() which does support asar paths on Linux, then pass the nativeImage object directly to BrowserWindow.

Issue #610 (forge v8 upgrade) is unrelated and tracked separately as noted in #614.

🐇 (ᵔᴥᵔ)