Skip to content

fix(desktop): resolve permission overlay assets in bundled dev - #2045

Merged
Astro-Han merged 2 commits into
apache:mainfrom
Colafornia:fix/permission-overlay-bundled-assets
Aug 4, 2026
Merged

fix(desktop): resolve permission overlay assets in bundled dev#2045
Astro-Han merged 2 commits into
apache:mainfrom
Colafornia:fix/permission-overlay-bundled-assets

Conversation

@Colafornia

Copy link
Copy Markdown
Contributor
English

Summary

In bundled desktop development, the macOS guided-permission action opens System Settings but never displays its overlay card.

dev.mjs bundles the main process into dist/main/main.js, while overlayAssetDir() assumed the tsc layout at dist/main/permission-overlay/permission-overlay-main.js. Its fixed ../.. traversal therefore resolved to the nonexistent apps/desktop/overlay directory instead of dist/overlay, so Electron could not load the overlay preload or HTML.

This PR:

  • resolves permission-overlay assets by walking up to the nearest dist directory, matching the existing cursor/PiP overlay pattern;
  • uses that resolver for both the preload and HTML paths;
  • covers the bundled dev and tsc production layouts with a focused regression test.

The getFileIcon() native crash is already fixed on main by #1920 and is not part of this PR.

Reproduction

On macOS, with Accessibility or Screen Recording permission ungranted:

  1. Run npm --workspace @maka/desktop run dev.
  2. Open Settings → Permissions.
  3. Click the guided grant action.

Before this change, System Settings opens but the permission overlay card does not appear. The bundled module resolves assets as follows:

Layout Module location Asset directory
tsc production dist/main/permission-overlay/permission-overlay-main.js dist/overlay
esbuild dev bundle dist/main/main.js apps/desktop/overlay (missing)

After this change, both layouts resolve to dist/overlay.

Verification

  • npm --workspace @maka/desktop run build:workspace-deps
  • npm --workspace @maka/desktop run build:main
  • node --test apps/desktop/dist/main/__tests__/permission-overlay-path.test.js — 1 passed
  • git diff --check origin/main...HEAD

Not run: the post-fix macOS permission-onboarding flow or the full repository test suite.

中文

概要

在 bundled desktop 开发环境中,macOS 的“引导授权”操作会打开系统设置,但权限引导卡片不会出现。

dev.mjs 将主进程代码合并到 dist/main/main.js,而 overlayAssetDir() 假设模块位于 tsc 输出路径 dist/main/permission-overlay/permission-overlay-main.js。固定的 ../.. 因此解析到不存在的 apps/desktop/overlay,而不是 dist/overlay,导致 Electron 无法加载 overlay 的 preload 和 HTML。

本 PR:

  • 向上查找最近的 dist 目录来解析权限浮层资源,与现有 cursor/PiP overlay 的实现方式保持一致;
  • 将该解析逻辑用于 preload 和 HTML 路径;
  • 添加聚焦回归测试,覆盖 bundled dev 与 tsc production 两种布局。

getFileIcon() 原生崩溃已由 main 上的 #1920 修复,不属于本 PR。

复现路径

在 macOS 上,保持“辅助功能”或“屏幕录制”权限未授权:

  1. 运行 npm --workspace @maka/desktop run dev
  2. 打开“设置 → 权限”。
  3. 点击“引导授权”。

修改前,系统设置会打开,但权限引导卡片不会出现。bundled module 的资源解析结果如下:

布局 模块位置 资源目录
tsc production dist/main/permission-overlay/permission-overlay-main.js dist/overlay
esbuild dev bundle dist/main/main.js apps/desktop/overlay(不存在)

修改后,两种布局均解析到 dist/overlay

验证

  • npm --workspace @maka/desktop run build:workspace-deps
  • npm --workspace @maka/desktop run build:main
  • node --test apps/desktop/dist/main/__tests__/permission-overlay-path.test.js — 1 项通过
  • git diff --check origin/main...HEAD

未运行:修复后的 macOS 权限引导手动流程及完整仓库测试套件。

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with a non-blocking suggestion

The fix is correct and follows the established seam. The dist-walk matches the cursor overlay's defaultOverlayDistDir() and PiP's defaultDistDir(), and the parameterized moduleUrl finally makes this walk-up logic unit-testable — the two existing copies have no test coverage at all, so this is a strict improvement.

One non-blocking observation: this is now the third byte-for-byte copy of the same walk-up resolver (cursor-overlay-window.ts, pip-electron.ts, and now permission-overlay-path.ts). That's the rule-of-three point. If you'd like, extracting a shared resolveOverlayAssetDir(moduleUrl = import.meta.url) into e.g. src/main/overlay-assets.ts and switching all three call sites would make the resolver a single source of truth — and the regression test you added would then cover the cursor/PiP paths for free, since they currently have zero coverage. Purely mechanical, low risk, and independent of this fix, so it can be a follow-up.

中文对照

批准,附带一个非阻塞建议

修复正确,且遵循了仓库既有的扩展点:向上查找 dist 的逻辑与 cursor overlay 的 defaultOverlayDistDir()、PiP 的 defaultDistDir() 一致;参数化的 moduleUrl 让这段 walk-up 逻辑第一次可以被单元测试——而现有的两份旧实现完全没有测试覆盖,因此这是净改进。

一个非阻塞观察:这已经是同一段 walk-up 解析器的第三份逐字节相同的复制cursor-overlay-window.tspip-electron.ts,以及现在的 permission-overlay-path.ts),恰好到了 rule-of-three 的提取时机。如果愿意,可以提取共享的 resolveOverlayAssetDir(moduleUrl = import.meta.url)(例如放到 src/main/overlay-assets.ts),三处调用点统一使用,这样你新增的回归测试也能顺带覆盖 cursor/PiP 路径——它们目前是零覆盖。这纯粹是机械替换、风险低,且与本次修复相互独立,可以作为 follow-up 单独做。

@Colafornia
Colafornia force-pushed the fix/permission-overlay-bundled-assets branch from 8642907 to 31f1262 Compare August 4, 2026 01:50

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve. The walk-up-to-nearest-dist resolver is correct for every current launch flow (bundled dev, tsc production, macOS dev app, asar), and the cursor/PiP refactor onto the shared resolver is behavior-preserving. Three notes for the record, none blocking: the no-dist fallback returns a silent wrong path (unreachable today, worth an existsSync check if the resolver is ever hardened); the test assertions are POSIX path literals (would break on a Windows runner, moot on the current CI matrix); the resolver diverges from the packaged-assets resourcesPath pattern, which only matters if overlay assets move to extraResources.

@Astro-Han
Astro-Han merged commit 1a4751f into apache:main Aug 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants