Skip to content

fix(desktop): make macOS dev permission grants reliable - #1920

Merged
Astro-Han merged 11 commits into
apache:mainfrom
liugddx:fix/dev-permission-overlay-native-icon-crash
Aug 3, 2026
Merged

fix(desktop): make macOS dev permission grants reliable#1920
Astro-Han merged 11 commits into
apache:mainfrom
liugddx:fix/dev-permission-overlay-native-icon-crash

Conversation

@liugddx

@liugddx liugddx commented Aug 2, 2026

Copy link
Copy Markdown
Member

Summary

  • avoid native bundle icon reads that can crash unpackaged Electron permission guides
  • generate a stable, ad-hoc-signed Maka Dev.app for macOS development
  • launch the development bundle through LaunchServices so TCC attributes permissions to Maka Dev, not the parent terminal
  • reuse the signed bundle until the installed Electron version changes
  • preserve Vite renderer HMR by explicitly forwarding its development URL
  • keep other platforms on their existing Electron development launcher
  • document the authorization and restart workflow
  • add regression coverage for LaunchServices arguments and environment filtering

Closes #1919

Root cause

The npm-installed Electron.app failed strict bundle signature verification. Skipping app.getFileIcon() prevented its native SIGTRAP, but dragging that bundle still did not produce a usable TCC grant. A valid ad-hoc signature and stable bundle ID were necessary but not sufficient when its internal executable was launched directly from a terminal: macOS continued returning Accessibility false and Screen Recording denied.

Launching the same signed bundle through LaunchServices made the TCC responsibility chain resolve to Maka Dev; the existing permission poll then changed to granted on the real macOS device.

Generated .app contents remain ignored and are never committed.

Verification

  • real macOS 13.4.1 authorization: Maka Dev appeared in System Settings and Maka changed from “waiting for permission” to granted
  • codesign --verify --deep --strict apps/desktop/.maka-dev/Maka\ Dev.app
  • npm run dev -- --user-data-dir=<isolated-dir> launched the signed bundle through LaunchServices with Vite HMR active
  • npm --workspace @maka/desktop run build
  • npm run test:scripts (23 passed)
  • focused permission/dev launcher tests (19 passed)
  • Biome checks and git diff --check

@liugddx

liugddx commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

Real-device follow-up: this patch successfully avoids the native SIGTRAP, but the development authorization flow remains incomplete.

After the dragged Electron entry is enabled in System Settings, the running Electron executable still observes Accessibility as false and Screen Recording as denied. codesign --verify --deep --strict reports that the npm-installed Electron.app has no code resources despite its signature requiring them.

I am converting this PR to Draft. The remaining work is to introduce a stable, validly signed development app bundle and ensure that the running executable and dragged TCC target are the same bundle. See the linked issue follow-up for the probe output and verification details.

@liugddx
liugddx marked this pull request as draft August 2, 2026 15:18
@liugddx liugddx changed the title fix(desktop): avoid dev permission icon crash fix(desktop): make macOS dev permission grants reliable Aug 2, 2026
@liugddx
liugddx marked this pull request as ready for review August 2, 2026 15:41
@Astro-Han

Copy link
Copy Markdown
Contributor

The main blocker is CI. The typecheck job is failing in Knip because the new launcher invokes osascript and plutil, but neither binary is listed in knip.json. The TypeScript checks themselves passed. Please register both binaries in ignoreBinaries and rerun CI.

The E2E job is also red on project-management.spec.ts. That failure does not appear related to this diff, but the required check still needs a successful rerun or a rebase if main already contains the fix.

A few other issues should be addressed:

  • The icon regression test only checks that shouldLoadNativeBundleIcon(false) returns false. Removing either guard around the real app.getFileIcon() calls would still leave the test green. Please test the observable invariant that getFileIcon() is never called for an unpackaged app.
  • The runtime tests cover the generated open arguments, but not bundle preparation or caching. The important cases are a cache hit, Electron/schema/bundle-ID changes, failed signature verification, and ensuring a failed build does not write a valid marker.
  • (marker.schemaVersion ?? 1) accepts a marker with no schema version as v1. This should be a strict comparison so an incomplete or old marker forces a rebuild.
  • Several comments and one test still describe Electron.app as the development TCC identity. The final implementation now runs and grants Maka Dev.app, so those references should be updated.

The signed Maka Dev.app plus LaunchServices approach matches the macOS TCC behavior found during real-device testing. I would keep that structure and fix the CI, tests, and stale documentation rather than redesigning the launcher.

@liugddx

liugddx commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

Thanks — addressed in c841d716:

  • added osascript and plutil to Knip ignoreBinaries; local npx knip --workspace apps/desktop now exits successfully
  • replaced the boolean icon-policy test with an observable invariant: the injected native loader is called zero times for unpackaged apps, and both production app.getFileIcon() paths now go through that guarded loader
  • added runtime-cache coverage for exact cache hits, Electron/schema/bundle-ID changes, failed signature verification, and marker omission after a failed build
  • changed schema matching to strict equality; missing schema versions now force rebuilds
  • updated stale comments/tests to identify generated Maka Dev.app as the supported development TCC identity

Real-device follow-up also exposed the Screen Recording restart path dropping launch arguments. The generated bundle now includes a small relaunch bootstrap and a stable isolated user-data path, so macOS “Quit & Reopen” returns to the repository app. Screen Recording onboarding now performs a real desktopCapturer request and, when still ungranted, continues into the existing drag card rather than replacing it.

Local verification:

  • desktop typecheck passed
  • Knip passed (only pre-existing configuration hints remain)
  • script suite passed (26 tests)
  • focused permission/runtime suite passed (24 tests)
  • Biome and git diff --check passed

The unrelated project-management E2E failure should rerun on this push.

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the thorough work here — the real-device validation is genuinely valuable, and the core direction is right. A dedicated, stable, signed Maka Dev.app launched through LaunchServices is exactly what macOS TCC requires: it gives the grant a stable identity (com.maka.dev), a verifiable signature, and a responsible process that is the app bundle rather than the terminal. Keeping the executable named Electron also correctly preserves app.isPackaged === false, so the dev-mode gates stay intact. I'd keep this architecture.

There is one structural issue I'd address before merging, plus a few smaller ones.

P1: open -W couples the app's lifetime to the dev session's — "Quit & Reopen" kills the session

The whole point of the relaunch bootstrap is the Screen Recording "Quit & Reopen" path, but today that path ends with a dead session:

  • open -n -W returns when the app quits. On any quit — including the system-initiated reopen — dev.mjs:184 treats it as the end of the dev session, shuts down Vite, and exits the supervisor. The relaunched app is then an orphan.
  • The bootstrap (dev-app-runtime.mjs:119-135) restores only appPath/userData. It doesn't restore VITE_DEV_SERVER_URL, so after reopen main-window.ts:197,391 falls back to dist-renderer/index.html, which a pure npm run dev checkout never builds → blank window, no HMR, and the env-based dev gates silently turn off.

From first principles: the dev session (Vite + env + args) and the app instance are two different lifecycles. The app may quit and restart at any time (user quit, crash, TCC reopen); the session should only end on Ctrl-C/SIGTERM.

Suggested direction (not a redesign — the bundle layer is fine):

  • Drop -W and don't key session teardown off the open process exiting; make dev.mjs a supervisor that lives for the whole dev session.
  • Have the bootstrap recover the session context from the still-alive supervisor (a per-worktree Unix socket, or a small session.json with viteUrl + supervisor PID validated before use) and restore VITE_DEV_SERVER_URL/env before importing main.js. Then a system reopen reconnects to the same Vite automatically, with no need to detect "was this quit a reopen".
  • This also replaces the --args/--env launch protocol — they only work for the first launch; the system reopen replays neither.

P2: environment-forwarding regression on macOS

LaunchServices doesn't inherit the shell environment; only vars explicitly passed via --env reach the app. Today that's just VITE_DEV_SERVER_URL (dev-app-runtime.mjs:32-35), so ANTHROPIC_API_KEY/OPENAI_API_KEY/MAKA_* that the old electron . inherited are silently lost on macOS dev runs. Options: forward a curated allowlist of the vars the app actually consumes, or document the limitation explicitly. The socket/session approach above is a natural place to restore them — without putting secrets on a command line.

Smaller items

  • Concurrent npm run dev: two processes race the .maka-dev rebuild (rmSync + ditto/codesign, no lock) and share com.maka.dev + userData, so the second instance can osascript-quit the first's running app. A lockfile around the rebuild and PID-scoped quit (instead of bundle-ID broadcast) would fix both.
  • quitMacosDevelopmentApp silently swallows osascript failure (it needs Automation TCC). If it fails, Ctrl-C leaves an orphan app, and the next dev run exits immediately on the single-instance lock. Check the exit status and fall back to PID kill.
  • --args + withDevelopmentUserData: the bootstrap doesn't read argv, so these are currently dead weight — and a user-supplied --user-data-dir is silently overridden by the hardcoded default. Either honor argv in the bootstrap or drop the forwarding.
  • Test coverage: the runtime-cache protocol and policy functions are well tested, but the PR's headline behavior — the request_screen_capture action and the IPC branch in permissions:startDragOnboarding — has no coverage, and the non-darwin guard, --user-data-dir dedup, and corrupt-runtime.json recovery are untested. Also, start-dev-app.mjs largely duplicates dev.mjs's launch/quit orchestration; a shared helper would be cleaner (asar is also an undeclared transitive dep — worth declaring explicitly).

Net: the architecture is right and close; fixing the session lifecycle (P1) and the env regression (P2) should be enough to make this mergeable. Happy to help verify on-device once the reopen path keeps Vite alive.

中文意见(简版)

核心结论:方向正确,不需要换方案——专属签名 Maka Dev.app + LaunchServices 启动正是 macOS TCC 的硬性要求(身份稳定、签名可验证、responsible process 是 app bundle 而非终端),保留 Electron 可执行文件名也让 isPackaged === false 的开发模式门保持生效。真正的问题是生命周期设计:

  1. P1:Quit & Reopen 会把开发会话弄死open -W 在 app 退出时返回,dev.mjs 把「app 被系统重启」误判为「开发结束」,关掉 Vite 退出;重开时 bootstrap 又没恢复 VITE_DEV_SERVER_URL,窗口回退到不存在的 dist-renderer/index.html → 白屏、无 HMR。第一性原理:app 实例和 dev 会话是两个生命周期,app 退出不该结束会话。建议 dev.mjs 改为 supervisor(只在 Ctrl-C/SIGTERM 时结束),bootstrap 通过 socket 或 session 文件向存活的 supervisor 恢复 dev URL 和环境,系统重开即可自动连回同一 Vite。
  2. P2:macOS 开发时环境变量静默丢失。LaunchServices 不继承 shell 环境,只有 --env 显式转发的变量能到达 app,目前仅 VITE_DEV_SERVER_URLANTHROPIC_API_KEY/MAKA_* 等全部丢失(旧 electron . 是完整继承的)。需要按 app 实际消费的变量做白名单转发,或明确文档化。
  3. 次要项:并发 npm run dev 无锁竞争 rebuild、且 bundle-ID 广播 quit 会误杀另一个 dev 的 app;osascript quit 静默失败会留下孤儿 app 并砖化后续 dev 运行;--args/--user-data-dir 转发是死代码且会静默覆盖用户参数;screen-recording 授权路径(action + IPC 分支)零测试覆盖;start-dev-app.mjsdev.mjs 启动/退出编排重复。

结论:架构正确、接近可合;修完 P1(会话生命周期)和 P2(环境转发)即可合并。

@liugddx

liugddx commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Thanks for the thorough work here — the real-device validation is genuinely valuable, and the core direction is right. A dedicated, stable, signed Maka Dev.app launched through LaunchServices is exactly what macOS TCC requires: it gives the grant a stable identity (com.maka.dev), a verifiable signature, and a responsible process that is the app bundle rather than the terminal. Keeping the executable named Electron also correctly preserves app.isPackaged === false, so the dev-mode gates stay intact. I'd keep this architecture.

There is one structural issue I'd address before merging, plus a few smaller ones.

P1: open -W couples the app's lifetime to the dev session's — "Quit & Reopen" kills the session

The whole point of the relaunch bootstrap is the Screen Recording "Quit & Reopen" path, but today that path ends with a dead session:

  • open -n -W returns when the app quits. On any quit — including the system-initiated reopen — dev.mjs:184 treats it as the end of the dev session, shuts down Vite, and exits the supervisor. The relaunched app is then an orphan.
  • The bootstrap (dev-app-runtime.mjs:119-135) restores only appPath/userData. It doesn't restore VITE_DEV_SERVER_URL, so after reopen main-window.ts:197,391 falls back to dist-renderer/index.html, which a pure npm run dev checkout never builds → blank window, no HMR, and the env-based dev gates silently turn off.

From first principles: the dev session (Vite + env + args) and the app instance are two different lifecycles. The app may quit and restart at any time (user quit, crash, TCC reopen); the session should only end on Ctrl-C/SIGTERM.

Suggested direction (not a redesign — the bundle layer is fine):

  • Drop -W and don't key session teardown off the open process exiting; make dev.mjs a supervisor that lives for the whole dev session.
  • Have the bootstrap recover the session context from the still-alive supervisor (a per-worktree Unix socket, or a small session.json with viteUrl + supervisor PID validated before use) and restore VITE_DEV_SERVER_URL/env before importing main.js. Then a system reopen reconnects to the same Vite automatically, with no need to detect "was this quit a reopen".
  • This also replaces the --args/--env launch protocol — they only work for the first launch; the system reopen replays neither.

P2: environment-forwarding regression on macOS

LaunchServices doesn't inherit the shell environment; only vars explicitly passed via --env reach the app. Today that's just VITE_DEV_SERVER_URL (dev-app-runtime.mjs:32-35), so ANTHROPIC_API_KEY/OPENAI_API_KEY/MAKA_* that the old electron . inherited are silently lost on macOS dev runs. Options: forward a curated allowlist of the vars the app actually consumes, or document the limitation explicitly. The socket/session approach above is a natural place to restore them — without putting secrets on a command line.

Smaller items

  • Concurrent npm run dev: two processes race the .maka-dev rebuild (rmSync + ditto/codesign, no lock) and share com.maka.dev + userData, so the second instance can osascript-quit the first's running app. A lockfile around the rebuild and PID-scoped quit (instead of bundle-ID broadcast) would fix both.
  • quitMacosDevelopmentApp silently swallows osascript failure (it needs Automation TCC). If it fails, Ctrl-C leaves an orphan app, and the next dev run exits immediately on the single-instance lock. Check the exit status and fall back to PID kill.
  • --args + withDevelopmentUserData: the bootstrap doesn't read argv, so these are currently dead weight — and a user-supplied --user-data-dir is silently overridden by the hardcoded default. Either honor argv in the bootstrap or drop the forwarding.
  • Test coverage: the runtime-cache protocol and policy functions are well tested, but the PR's headline behavior — the request_screen_capture action and the IPC branch in permissions:startDragOnboarding — has no coverage, and the non-darwin guard, --user-data-dir dedup, and corrupt-runtime.json recovery are untested. Also, start-dev-app.mjs largely duplicates dev.mjs's launch/quit orchestration; a shared helper would be cleaner (asar is also an undeclared transitive dep — worth declaring explicitly).

Net: the architecture is right and close; fixing the session lifecycle (P1) and the env regression (P2) should be enough to make this mergeable. Happy to help verify on-device once the reopen path keeps Vite alive.

中文意见(简版)

Done. PTAL

@Astro-Han

Astro-Han commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This rework addresses the core issue well — thank you. The session-file mechanism correctly separates the app-instance lifecycle from the dev-session lifecycle, and dropping -W, the PID-scoped shutdown, the rebuild lock, and declaring @electron/asar are all right moves. Factoring the screen-recording flow into testable pure functions is a nice improvement too. I verified locally: node --test apps/desktop/scripts/dev-app-runtime.test.mjs passes 6/6.

This comment updates the previous review with verification results and a few additional findings. CI is currently red on format:check, and these gaps remain before merge:

Must fix — CI is red (format:check)

npm run format:check fails on knip.json. Removing electron-builder from ignoreDependencies left a 2-element array in multiline form; Biome only accepts multiline when the array doesn't fit on one line, so it now requires the single-line form. Please run npm run format (or collapse the array) and push — the typecheck job gates on this.

P2 — env allowlist misses vars the app actually consumes

A full sweep of process.env reads in apps/desktop/src confirms the gap set: GH_TOKEN/GITHUB_TOKEN (read alongside COPILOT_GITHUB_TOKEN at github-copilot-subscription-service.ts:178) and RIVE_BIN (rive-workflow-tool.ts:121 / rive-cli.ts:181; MAKA_RIVE_BIN is covered via the prefix). Devs who only set GH_TOKEN or RIVE_BIN silently lose them on macOS.

P2 — concurrent dev across worktrees still fails silently

Confirmed empirically: all worktrees share ~/Library/Application Support/Maka Dev, so Electron's single-instance lock (main.ts:21) makes the second worktree's app exit 0 — while its supervisor ignores open's exit code and keeps Vite running, so the second dev looks successful but has no window. Worth: per-worktree user data, or an explicit early failure with a diagnostic.

P2 (new) — a runtime rebuild wipes the active session's state, orphaning the running app

session.json and app.pid live inside .maka-dev/, which prepareDevelopmentApp's reset removes (rmSync(DEV_RUNTIME_DIR, …)). A rebuild while a dev session is active (Electron upgrade, schema bump, prepare:dev-app) deletes the supervision state of the still-running app: the supervisor can no longer quit it (an orphan holding the single-instance lock, so subsequent dev launches exit immediately), and a later TCC "Quit & Reopen" finds no session → broken/blank window. Consider keeping session/pid state outside the wiped directory, or moving the session-ownership check ahead of the rebuild.

P2 (new) — app.pid is written before the single-instance lock check

The bootstrap writes app.pid during module evaluation, then imports main.js, where main.ts:21 fails the lock and exits 0. So a second/relaunched instance clobbers app.pid with a pid that dies immediately: after a supervisor crash, the next supervisor can no longer terminate the leftover live app (must Cmd-Q by hand). Writing app.pid only after the lock is acquired would fix it.

P2 (new) — app boot failures are invisible to the supervisor (silent hang)

open exits 0 at LaunchServices handoff and the app's stderr goes to the system log, so a bootstrap/main crash (missing dist, stale paths, corrupt bundle) is invisible: npm start idles forever via the keepalive timer, npm run dev keeps Vite alive with no output — a regression vs. the old electron . (inherited stdio). A launch deadline / liveness poll on app.pid, or a bootstrap→supervisor failure report, would surface it.

P3 (updated) — graceful shutdown

Correction to the previous review: on this Electron (43.x), a process-level SIGTERM does trigger the before-quit cleanup path (app-lifecycle.ts:373-410), verified on-device. The only residual is the hard 500 ms SIGTERM→SIGKILL cap racing that async cleanup — worth widening to a few seconds with SIGKILL as a longer last resort.

P3 (nice-to-have)

  • The bootstrap still embeds the absolute DESKTOP_DIR, and the runtime marker doesn't include it — moving/renaming the repo hits the cache and the bootstrap imports the old path.
  • .maka-dev.lock isn't gitignored; a crashed supervisor leaves an untracked file.
  • macOS dev silently drops extra Electron CLI flags (--remote-debugging-port=…, --enable-logging) since open args are fixed; only --user-data-dir= is honored via the session.

Test coverage note

The runtime-cache and policy tests are solid, but the session.json protocol (write/read/clear), quitMacosDevelopmentApp, the rebuild lock, and the 'already supervised' rejection have no behavioral tests — and the generated bootstrap is never parsed or executed, so template-syntax breakage would only surface on a real Mac. These are pure Node and testable with temp dirs if the runtime dir / platform are injectable.

Net: the architecture is right and the happy path works; closing the format check (required) plus the env allowlist, per-worktree isolation, state-survival across rebuild, and the pid/lock ordering should make this mergeable. Happy to re-verify after the next push.

中文意见(简版)

这轮重构方向正确——session 机制、去 -W、PID 退出、asar 声明、screen-recording 纯函数化都到位;本地测试 6/6 通过。本条为更新版(含验证结果与新发现),合并前还需收尾:

  1. 必改:CI 已红format:check 挂在 knip.json(删 electron-builder 后 2 元素数组需按 Biome 规则改单行)。npm run format 即可。
  2. P2:env 白名单。全量 grep 确认完整缺口 = GH_TOKEN/GITHUB_TOKEN/RIVE_BIN,其余无遗漏。只设这些变量的开发者会静默丢失。
  3. P2:跨 worktree 并发(已实证)。共享 userData → 单实例锁让第二个 worktree 的 app 退出 0,supervisor 却继续跑 Vite → 「看起来成功、实际无窗口」。建议 per-worktree userData 或明确早期报错。
  4. P2(新):rebuild 抹掉活跃会话状态session.json/app.pid 在被 rmSync.maka-dev/ 内——升级 Electron 或跑 prepare:dev-app 时,活跃 app 变成无法 quit 的孤儿(占着单实例锁、砖化后续 dev),Quit&Reopen 也因无 session 白屏。建议状态移出被清目录,或把会话占用检查前置。
  5. P2(新):app.pid 先于单实例锁写入。第二实例退出 0 前会覆盖 pid 记录 → supervisor 崩溃后,新 supervisor 无法终止遗留的活 app(只能手动 Cmd-Q)。应在锁获取后再写。
  6. P2(新):app 启动失败静默挂起open 立即返回 0、app stderr 进系统日志 → bootstrap 崩溃时 npm start 无限挂起、npm run dev 无输出。需要启动时限/liveness 探测。
  7. P3(修正):优雅退出。实测 Electron 43 上 SIGTERM 会走 before-quit 清理(更正上条评论的说法);仅需把 500ms SIGKILL 时限放宽到数秒。
  8. P3:bootstrap 绝对路径未进缓存键(repo 移动后加载旧路径);.maka-dev.lock 未 gitignore;macOS 下额外 Electron CLI flags 静默丢弃。
  9. 测试:session 协议/quit/锁/拒绝路径零行为测试,生成的 bootstrap 从未被执行过;这些是纯 Node,把 runtime 目录/platform 做成可注入即可在临时目录测。

结论:架构正确、happy path 已通;收掉格式检查(必须)+ env 白名单 + 跨 worktree 隔离 + rebuild 状态保全 + pid 写入顺序即可合并。

gdliu3 and others added 5 commits August 3, 2026 15:36
The knip.json ignoreDependencies array was split across multiple lines,
but the two entries fit within Biome's 100-char lineWidth, so
"biome format" (the CI check-mode gate) wants them collapsed onto one
line. Collapse the array to clear the format check.
…lent launch failures

Several defects in the macOS dev app supervisor and runtime, all on the
same session/launch lifecycle:

- Write app.pid only after winning the single-instance lock. The
  bootstrap previously wrote it before acquiring the lock, so a losing
  second instance clobbered the winner's pid record and left the live
  app as an orphan the supervisor could no longer quit.

- Isolate userData per linked git worktree. A shared profile made
  Chromium's single-instance lock treat a second worktree's app as a
  duplicate: it exited 0 while the supervisor kept serving Vite, giving
  "looks launched, no window". The primary checkout keeps its historical
  profile; linked worktrees (.git is a file) get a stable per-checkout
  profile. The bundle id stays com.maka.dev so TCC identity is unchanged.

- Move session/pid state to a sibling dir that survives a runtime
  rebuild, and refuse a rebuild while a live app owns the lock. A rebuild
  rmSync'd the whole runtime dir, wiping the session and orphaning any
  running app; the state now outlives the rebuild and an occupied
  runtime fails early and legibly instead.

- Probe for a live app after launch instead of trusting `open`'s exit
  code, which returns 0 the moment LaunchServices accepts the request. A
  crashed bootstrap now fails loudly rather than hanging the supervisor
  on its keep-alive timer with no window and no error.

- Pin desktopDir in the runtime cache marker (schema 4 -> 5). The
  relaunch bootstrap bakes in an absolute path, so a moved repo must
  rebuild instead of loading a stale bootstrap.

- Forward GH_TOKEN, GITHUB_TOKEN and RIVE_BIN through the curated dev
  environment (all consumed by dev tooling), and widen the
  SIGTERM->SIGKILL grace from 500ms to 3s so before-quit cleanup can
  finish.
…overlay-native-icon-crash

# Conflicts:
#	package.json
…e-icon-crash' into fix/dev-permission-overlay-native-icon-crash

# Conflicts:
#	apps/desktop/.gitignore
#	apps/desktop/scripts/dev-app-runtime.mjs
#	apps/desktop/scripts/dev-app-runtime.test.mjs
#	apps/desktop/scripts/start-dev-app.mjs

@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.

A scoping suggestion, nothing blocking

First, the core of this PR is right. The Electron.app from npm has a broken code seal, which is the real reason both the icon crash and the TCC grants fail. Generating a stable, ad-hoc signed Maka Dev.app and launching it through LaunchServices attacks that root cause directly. Commits 1-3 are the correct fix and should stay.

The rest of the diff is a different project. The probe, the launch-status file, the session protocol, and the main.ts reporting all exist to supervise a dev process. That is a real concern, but it is not what #1919 asked for, and it is where the false-failure modes come from (quit within 30s, slow boot, the hardcoded timeout).

A much smaller shape that still fixes the issue completely:

flowchart TB
    subgraph keep["keep (the actual fix)"]
        A1["crash guard: skip getFileIcon unpackaged (+28)"]
        A2["stable signed Maka Dev.app, cached per electron version"]
        A3["launch through LaunchServices (open -a)"]
        A4["screen recording consent via desktopCapturer"]
    end
    subgraph drop["drop or shrink"]
        B1["probe + launch-status + main.ts reporting (8973abd5)"]
        B2["full session protocol: env allowlist, pid lock, schema"]
    end
    A2 --> A3 --> A4
    B1 -. "replaced by open -W" .-> A3
Loading

Three changes carry most of the weight:

  1. open -W instead of open. It blocks until the app exits, so user quit and crash both come back as a normal exit. The probe, the status file, and the main.ts ready/failed reporting all disappear. The 30s timeout and its two false-failure modes cannot exist in this shape.
  2. Let the dev bootstrap write its own pid file (one line). main.ts goes back to zero changes. Production startup should not know a dev harness exists.
  3. Shrink session persistence to a single field, the Vite dev URL. That is the only thing HMR needs after a system relaunch. The env allowlist and ownership tracking are speculative until someone actually runs two dev instances at once.

Rough target: around 450-500 lines instead of 1122, with main.ts untouched. The commit history would also need a rebase to drop the two merge commits and the duplicated push.

If the supervision layer matters on its own (the worktree userData isolation and the rebuild guard in 8f3e1d1 do look useful), it reads better as a separate PR with its own rationale. It does not need to ride along with the permission fix.

One trade-off worth deciding explicitly: after open, the app's stdout/stderr no longer reach the terminal. open --stdout <file> --stderr <file> plus a tail -f in the launcher is a small compensation, or the README can point at log stream --process Electron.

中文说明

先说结论:这个 PR 的核心方向是对的。npm 里的 Electron.app 签名是破损的,这正是图标崩溃和 TCC 授权都失效的真正原因;生成一个稳定签名的 Maka Dev.app 并用 LaunchServices 启动,是在修根因。前 3 个 commit 是正确的修复,应该保留。

膨胀的部分是后面的探针、launch-status 文件、session 协议和 main.ts 上报,这些属于"开发进程监督",不是 #1919 的职责,也正是 30 秒超时、快速退出误报这些新故障的来源。

三个改动能省下大部分代码:用 open -W(等到 app 退出才返回,探针整层可以删掉)、让 bootstrap 自己写 pid(main.ts 零改动)、session 只保留 Vite URL 一个字段(重启后只有它需要恢复)。

这样大概能从 1122 行缩到 450-500 行。如果你觉得 worktree 隔离和 rebuild 保护(8f3e1d10)有价值,它们更适合单独开一个 PR,有独立的理由再评审。这只是建议,你可以按自己的判断来。

@liugddx

liugddx commented Aug 3, 2026

Copy link
Copy Markdown
Member Author
image

M4n5ter pushed a commit that referenced this pull request Aug 22, 2026
… macOS builds (#3455)

app.getFileIcon(path, { size: 'large' }) is unsupported on macOS and hits
a fatal NOTREACHED inside Chromium's IconLoader — the process dies with
SIGTRAP before the promise settles, so loadNativeBundleIcon's try/catch
never runs. Packaged builds crashed the moment the drag-to-grant
permission guide loaded the app icon; dev builds were spared only by the
isPackaged gate from #1920.

Hoist the size choice into one shared BUNDLE_ICON_OPTIONS constant
requesting 'normal' (32x32, supported everywhere), which the existing
resize step upscales to the same 64x64 the code already produced.

Fixes #3352

Generated-by: Claude Code
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.

fix(desktop): macOS development permission guide crashes or cannot grant access

2 participants