Skip to content

feat(desktop): replace native recovery dialogs - #4474

Open
M4n5ter wants to merge 34 commits into
fix/session-share-peer-route-livenessfrom
feat/maka-browser-recovery-dialogs
Open

feat(desktop): replace native recovery dialogs#4474
M4n5ter wants to merge 34 commits into
fix/session-share-peer-route-livenessfrom
feat/maka-browser-recovery-dialogs

Conversation

@M4n5ter

@M4n5ter M4n5ter commented Sep 1, 2026

Copy link
Copy Markdown
Member
English

Summary

  • Replace product-owned Electron message boxes with a compact, Maka-styled temporary BrowserWindow, including workspace repair, Runtime Host recovery and replacement, client-settings confirmation, Renderer recovery, fatal startup, and development second-instance diagnostics.
  • Keep the main window optional during startup: the temporary surface can render after Electron is ready even when the main Renderer has not opened. A native message box remains only as a last-resort fallback when the BrowserWindow cannot become interactive.
  • Reload the existing main window after a Renderer failure and wait for its first committed paint, preserving the Desktop process, Runtime Host, and background services. Runtime Host flows keep their existing process boundary: they repair or replace only the Host and then continue startup. Updater quitAndInstall behavior remains unchanged because installing a new binary requires an application restart.
  • Reduce Runtime Host interruption prompts conservatively: only a complete activity snapshot proving zero connections, operations, and residencies permits silent replacement. Active or unverifiable work requires explicit confirmation and defaults to cancellation.
  • Generate the dialog's design-token asset from the canonical Maka/Astryx sources, share the core wordmark authority, and resolve persisted locale, theme, and palette before post-settings dialogs.
  • Preserve native open/save panels and operating-system-owned surfaces such as permissions, System Settings, Finder, external browsers, and notifications.

Screenshots

Runtime Host idle status unknown · dark · Chinese Workspace repair · light · English
Runtime Host replacement dialog with unknown idle status in dark theme and Chinese Workspace repair dialog in light theme and English
Renderer recovery · Nord dark · English Settings confirmation · Coral light · Chinese
Renderer recovery dialog using the Nord palette in dark theme and English Settings confirmation dialog using the Coral palette in light theme and Chinese

Verification

  • npm run build:test
  • npm run format:check
  • npm run lint
  • Full workspace typecheck, npm run check:asf-headers, Renderer architecture check, and the Runtime Host protocol-epoch guard
  • Affected Desktop, CLI, and real Runtime Host kernel suites: 163 passed, 0 failed
  • Full Desktop suite: 1,890 passed out of 1,892. The two existing Electron/OpenSSL environment failures are the TUF and Sigstore ECDSA attestation cases (ERR_OSSL_EVP_NO_DEFAULT_DIGEST under Electron Node 24.18.1); no affected test failed.
  • Browser presentation and recovery coverage exercises parent/modal selection, response navigation, close-to-cancel mapping, renderer failure and timeout, native fallback, process-lifetime retention, first-paint reveal, late-ready suppression, and window destruction.
  • Chromium visual verification covered the four production-HTML scenarios above; Electron probes covered standalone-window lifetime and CSP enforcement.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex — implementation, tests, iterative parallel review, visual verification, screenshots, and PR preparation.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No
中文

概要

  • 将产品自身触发的 Electron 原生消息框替换为紧凑、符合 Maka 设计的临时 BrowserWindow,覆盖工作区修复、Runtime Host 恢复与替换、客户端设置确认、Renderer 恢复、启动失败和开发环境重复实例诊断。
  • 启动阶段不要求先渲染主窗口:Electron ready 后,即使主 Renderer 尚未打开,也能显示临时界面。只有 BrowserWindow 无法进入可交互状态时才回退到原生消息框。
  • Renderer 异常时复用原窗口重新加载,并等待首个已提交画面;Desktop 进程、Runtime Host 与后台服务均保持不变。Runtime Host 流程维持原有进程边界:只修复或替换 Host,然后继续启动。更新器的 quitAndInstall 保持不变,因为安装新二进制确实需要重启应用。
  • 在保证安全的前提下降低 Runtime Host 提示频次:只有完整活动快照明确证明连接、操作和驻留项均为零时才静默替换;存在活动或无法确认时必须显式确认,并默认取消。
  • 从 Maka/Astryx 权威来源生成对话框设计 token 资产,共用 core 层字标,并在设置可用后的弹窗展示前解析持久化语言、主题和配色。
  • 保留原生打开/保存面板,以及权限、系统设置、Finder、外部浏览器和通知等由操作系统负责的界面。

截图

Runtime Host 空闲状态未知 · 深色 · 中文 工作区修复 · 浅色 · 英文
深色中文 Runtime Host 空闲状态未知替换对话框 浅色英文工作区修复对话框
Renderer 恢复 · Nord 深色 · 英文 设置确认 · Coral 浅色 · 中文
Nord 深色配色下的英文 Renderer 恢复对话框 Coral 浅色配色下的中文设置确认对话框

验证

  • npm run build:test
  • npm run format:check
  • npm run lint
  • 全 workspace typecheck、npm run check:asf-headers、Renderer architecture check 与 Runtime Host protocol epoch guard
  • 受影响的 Desktop、CLI 与真实 Runtime Host kernel 测试:163 通过、0 失败
  • Desktop 全量测试:1,892 项中 1,890 项通过。两个既有的 Electron/OpenSSL 环境失败来自 TUF 与 Sigstore ECDSA attestation(Electron Node 24.18.1 下的 ERR_OSSL_EVP_NO_DEFAULT_DIGEST);受影响测试均未失败。
  • Browser presentation 与恢复边界覆盖父窗口/modal 选择、response navigation、关闭映射到取消、renderer 失败与超时、原生兜底、进程生命周期保留、首帧 reveal、late-ready 抑制与窗口销毁。
  • Chromium 视觉验证覆盖上方四个生产 HTML 场景;Electron probe 覆盖独立窗口生命周期与 CSP。

AI 使用

选择一项(权威勾选状态见英文版):

  • ☐ 没有生成式工具作出实质贡献
  • ☑ 生成式工具作出了实质贡献

工具及范围:OpenAI Codex — 实现、测试、循环并行审查、视觉验证、截图与 PR 整理。

检查清单

  • ☑ 测试覆盖该变更,并会在缺少该变更时失败
  • ☑ lint、format、typecheck 与受影响测试均在本地通过

此 PR 是否改变行为?

  • ☑ 是 — 已在上方概要中说明
  • ☐ 否

@M4n5ter
M4n5ter force-pushed the feat/maka-browser-recovery-dialogs branch from 4632e00 to 6c1485c Compare September 1, 2026 13:22

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved at exact head 6c1485cdb012489eacecd5a19ea0d478c74ab23d at the explicit direction of M4n5ter. No technical review was performed as part of this action; M4n5ter requested the approval and accepts responsibility for subsequent handling.


Review notice: This approval was submitted by an automated review agent operated by WAWQAQ and is published at the direction of M4n5ter, who requested this action and is the human accountable for it.

@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Sep 1, 2026

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

Reviewed 6c1485cd. The direction is right — a product-owned dialog that lives inside Chromium is testable and inspectable in ways a native message box never is, and keeping open/save panels and OS permission surfaces native is the correct line to draw. The screenshots look good.

Almost everything I found traces to one shape, so I've grouped the comments that way rather than filing them separately. In five places an authority already existed and a narrower or looser copy was written beside it — and in every one of them the correct value was already in hand at the call site. Two of the five are visible in this diff as your own local patches:

Existing authority The copy Where the fact already was
dialog.showMessageBox's window-all-closed behaviour patched at one call site, main.ts:97-101 you wrote it down in that comment
the same helper's parent-visibility requirement patched at one call site, runtime-host-boot.ts:446 same
maka-tokens.css + astryx-theme/maka.css 36 lines of redeclared tokens appearance.palette sits on the object main-window.ts:284 already reads, two fields from the .theme it uses
desktopLocale a \p{Script=Han} sniff every call site had already resolved the locale
the idle test at runtime-host-desktop-manager.ts:925 a looser one at :953 result.handshake?.activity is right there, and formatActivity already renders it

You hit the dialog-helper seam twice and patched the call site both times. That is the signal to move the rule into the helper instead of writing a third patch — and doing that deletes the two patches you already wrote. Each of the five has the same property: fixing it at the owner removes code rather than adding it, which is what turns this from a net-addition PR into one that also retires something.

Findings: P1 ×3 (two in the dialog helper's contract, one in the idle test), P2 ×3, P3 ×4. The P1s are worth landing before merge; nothing here is a release blocker. Grading note: I considered the dialog auto-quit a release blocker at first and stepped it down — it makes startup recovery unusable and is a regression against the native dialogs it replaces, but there is no data loss and relaunching recovers.

Two items outside that root, both small:

  • runtime-host-boot.ts:450if (decision === "recover" && reloadMainRenderer()) return; app.quit();. When reloadMainRenderer() returns false (window or webContents already destroyed, shutdown signal aborted, reload() throws) the app exits silently, while the dialog copy promises "recovers the interface without restarting Maka". This PR also removes the last app.relaunch() in the Desktop app, so there is no longer a degraded restart to fall back to. Either say so before quitting, or relaunch. P2.
  • The summary says Runtime Host actions "no longer relaunch the whole Desktop process". On main the Runtime Host paths never called app.relaunch() — the only call was the renderer one this PR replaces. Worth correcting so the real change (the silent-replace policy) gets read on its own terms. P3.

One thing I went looking for and did not find, recorded so you don't have to wonder. I expected a parser or injection gap in the HTML construction and there isn't one. Every call-site-controlled value (title, message, detail, button labels) lands in a text node — <title> RCDATA, <h1>, .message, .detail — and escapeHtml's five characters are complete for those contexts; nothing reaches an unquoted attribute, a URL attribute, <style>, or a JS string. The unescaped interpolations are module constants, the messageBoxType allow-list, the nonce, and validButtonId-checked integers. I also confirmed empirically on the Electron version pinned here that the meta CSP is enforced under the opaque data: origin: an un-nonced inline script and an inline onerror= handler are both blocked while the nonced script runs. So a forged maka-dialog://response/<n> is not reachable, and no one can turn Cancel into Replace. The one residual is P3-shaped and noted inline.

Evidence boundary: I read the nine production files on this head, the dialog call sites and their position in the top-level startup sequence, main-window.ts, app-quit-coordinator.ts, and the host-side idle predicate in packages/runtime-host/src/server/host-kernel.ts, and diffed the token values against maka-tokens.css and astryx-theme/maka.css. I ran two standalone Electron probes for the auto-quit and CSP behaviour and extracted the built-in window-all-closed listener from the pinned Electron binary. I did not run this repo's suites or E2E, did not build the app, and did not verify the Windows or Linux behaviour on those platforms — the non-darwin reasoning is from reading runtime-host-boot.ts:1836. The four screenshots are hosted on a commit outside this branch, so I could not check them against the code.

Also, unrelated to the change: Release Windows check / package is red on qualify-released-cli-state-root.test.mjs:195, which touches none of these 14 files and looks pre-existing.


AI-assisted review: drafted with Maka; I verified the startup ordering, the token drift, the font-stack and wordmark duplication, and the two idle predicates against the branch source myself, and ran the Electron probes on my own machine.

Comment thread apps/desktop/src/main/browser-message-box.ts Outdated
Comment thread apps/desktop/src/main/browser-message-box.ts Outdated
Comment thread apps/desktop/src/main/browser-message-box.ts Outdated
Comment thread apps/desktop/src/main/browser-message-box.ts Outdated
Comment thread apps/desktop/src/main/runtime-host-desktop-manager.ts
@M4n5ter

M4n5ter commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

Also addressed the two non-inline review-summary notes:

  • A failed in-place Renderer reload now stays in recovery and prompts again. A successful retry reloads the same BrowserWindow and reveals it only after first-paint readiness from the exact main frame committed by that retry; Desktop is not relaunched.
  • The PR summary now distinguishes this new Renderer recovery behavior from the existing Runtime Host process boundary, and explicitly keeps updater quitAndInstall as the binary-install exception.

After synchronizing current main, the shared non-wire Host idle helper/export is explicitly declared compatible at epoch 88; no codec or message shape changed, and the protocol guard passes. The temporary dialog document loader is also explicitly approved by the existing secondary-window architecture boundary. The iterative deep-code-review and simplify-audit loop converged at 8319e4d24ab2939a2076df3f4b2f6da374aa5c6a with zero confirmed findings.

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

Approving. I read the follow-ups against the review summary notes and they hold: a failed in-place Renderer reload now stays in recovery and prompts again rather than reporting success, and the retry reloads the same BrowserWindow instead of building a second authority beside it.

Nothing further from me on the change itself.

One thing that isn't yours: the test job's Guard the protocol compatibility epoch step went red at 16:41 because #4460 merged and took epoch 88, and this branch is still on 88. 8319e4d2 hasn't moved it, so the guard will fail again on this run. Bumping to 89 clears it — though #4379 and #4184 are both sitting on 89 too, so it's worth checking main again right before merge rather than picking a number now.

…overy-dialogs

# Conflicts:
#	apps/desktop/src/main/__tests__/native-diagnostic-dialog.test.ts
#	apps/desktop/src/main/native-diagnostic-dialog.ts
#	apps/desktop/src/main/runtime-host-boot.ts
…eness' into feat/maka-browser-recovery-dialogs

# Conflicts:
#	apps/desktop/src/main/runtime-host-desktop-manager.ts
@M4n5ter
M4n5ter changed the base branch from main to fix/session-share-peer-route-liveness September 2, 2026 00:41
…eness' into feat/maka-browser-recovery-dialogs

# Conflicts:
#	apps/desktop/src/main/runtime-host-desktop-manager.ts
…eness' into feat/maka-browser-recovery-dialogs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants