fix(desktop): route every e2e window reveal through the reveal gate

[Astro-Han]

Summary

Follows up #4579. That PR resolved one reveal mode for the run — hidden, inactive, active — and routed the main window's reveal gate and the dock rule through it. It did not reach the windows other subsystems reveal on their own, so a local suite run still lost the foreground: every WorkHub summon or dock called show() + focus() on the floating panel and on Desktop, and the startup progress window did the same whenever a Runtime Host handoff asked for attention. Those calls live in workhub-presentation.ts and startup-progress-window.ts, neither of which could see the mode — the authority existed and simply was not consulted.

The already-resolved mode is threaded into both: WorkHubPresentationDeps takes it from runtime-host-boot.ts alongside the main window controller, and createStartupProgressWindow takes it from startup-presentation.ts, which resolves it for its branding decision already. The gate's private focusNow becomes the exported focusWindow, so the reveal-and-activate rule keeps one implementation; showWindowInactive covers the reveals that are deliberately quiet even in the product (WorkHub's progress card, the startup window's ordinary appearance) and that only had to learn to stay hidden. No new flag and no second resolution.

After review, the startup message box is gated too — see the disposition table.

Disposition of every reveal outside the gate

Site	Disposition
workhub-presentation.ts — summon, dock, progress card	Gated. focusWindow / showWindowInactive with deps.revealMode.
startup-progress-window.ts — did-finish-load, handoff attention, focus()	Gated. focusWindow / showWindowInactive with input.revealMode.
browser-message-box.ts:226 — dev single-instance loser dialog (main.ts), fatal-startup diagnostic box (main.ts)	Gated in this PR. The raw win.show(); win.focus(); now goes through focusWindow(win, appearance.revealMode); main.ts supplies startupRevealMode(), exported from startup-presentation.ts (all three of its inputs, app.isPackaged included, read correctly before ready), and runtime-host-boot.ts supplies the mode it already resolved. shouldShowLoserDialog never consulted the e2e flags, so both boxes appeared on screen in a hidden run.
cursor-overlay-window.ts:388, pip-window.ts:622, permission-overlay-controller.ts:217 — showInactive()	Left alone. No current spec reaches them, and an inactive reveal cannot take the foreground, which is the property this PR defends. Gating them would add threading for no observable difference in any mode.

Under hidden the message box is created and loaded but never revealed. Because nobody can answer a dialog that is not on screen, it settles as its cancel id instead of leaving the caller pending forever — without that, the dev loser instance would never reach its app.exit. In production the mode is active, so the dialog behaves exactly as before.

BrowserMessageBoxAppearance splits into the theme the rendered document consumes (locale, palette, dark) and the reveal mode, so the HTML builder keeps its narrow input and every caller of the window path has to answer the reveal question.

What actually differs in production

Not "unchanged" — three differences, all in the safe direction:

1. focusWindow adds an isDestroyed() guard that the inline WorkHub calls lacked.
2. focusWindow adds isMinimized() / restore() on the startup window's did-finish-load attention branch, which previously showed and focused a minimized window without restoring it.
3. showWindowInactive inherits showWindowOnceReady's isVisible() early return, where the old progress card called showInactive() on every progress-ready.

Hosted CI is unaffected: those runs are inactive on Xvfb, where the window is on screen either way.

Two consequences worth stating outright:

• Under hidden the handoff window is created and loaded but never shown, so a handoff prompt in a hidden run cannot be answered by a human. That is intended — hidden is the automated-run mode.
• detach still calls focusComposer(), which ends in view.webContents.focus() after an inactive reveal. That is web-contents focus, not window activation, and it early-returns when the parent is not visible. It is the one later call that could in principle re-enter the foreground question, and it is recorded here rather than changed.

The ~1 s foreground blip measured below (2 of 60 samples) is unmeasured, not located, and out of scope. The reviewer's reading is pre-ready application activation: app.dock.hide() runs inside showDesktopStartupProgress, after app.whenReady(), so the accessory policy is applied later than the activation it would suppress. Fixing that means applying the policy earlier, which is a separate change.

Merge

Merged origin/main. The only conflict was workhub-presentation.ts: main's #5153 made the WorkHub renderer lazy and moved the summon's reveal out of detach() into focusComposer(), where a cold summon now waits for the renderer to mount its composer. Resolved by keeping main's placement and this branch's authority — the reveal focusComposer() performs is focusWindow(parent, deps.revealMode), and detach() no longer reveals. Every reveal in the merged file goes through the gate, and the lazy-creation path adds no new ungated show() / focus(). The reveal-mode test was updated to assert nothing is revealed before ready and the mode-specific reveal after it.

Verification

• apps/desktop npm run typecheck and npx biome lint on the changed files — clean.
• apps/desktop npm run test:dist — 2496 pass, 0 fail, including window-reveal, workhub-presentation, startup-progress-window, main-startup-lifetime, workhub-control and workhub-runtime (49 pass across that subset).
• The new message-box case asserts that hidden neither shows nor focuses the dialog and active does both. Replacing focusWindow with the old win.show(); win.focus(); fails it. The three WorkHub/startup cases assert, per mode and per fixed path, that hidden shows and focuses nothing, inactive reveals with showInactive() and never focuses, and active shows and focuses exactly as before; reverting the source change fails all three. Note that workhub-presentation.test.ts rebuilds from src/ with esbuild while startup-progress-window.test.ts runs compiled output, so an ablation has to be made in src/.
• E2E, macOS, npx playwright test --config e2e/playwright.config.ts e2e/workhub-layout.spec.ts — 2 passed (before the merge). While it ran, the frontmost application was sampled every 0.5s the way fix(desktop): keep e2e windows visible without stealing focus #4579 measured it: 2 of 60 samples were Electron; the developer's own app held the foreground for the rest. Sampling a second green run gave 2 of 69. Playwright has not been re-run since the merge; CI owns it.
• Two earlier runs of this spec failed on stray characters appended to the composer draft ("…conversation.里y", "…conversation.额w"), which is physical keystrokes from the machine's human landing in the window mid-run, not a behavior change: the same build then passed the spec twice in a row, and the varying stray text differs run to run.

AI use

Select exactly one:

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

Tool(s) and scope: Claude Code — investigated the remaining reveal paths, wrote the change and its tests, and ran the verification above. Reviewed and submitted by a human contributor.

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

[jackwener]

APPROVE, bound to bb067b55ce473393052dc718d1b943017cdbaecf. One P3 inline; nothing P0–P2. Hosted test and label are terminal green on this exact head.

One merge-readiness fact first, since the brief did not mention it: GitHub reports this PR as CONFLICTING against its base. CI being green does not cover that. Nothing to do for the review; flagging it so the state is not a surprise later.

Taking the review points in order.

Every e2e-reachable reveal is gated — with one boundary worth stating precisely. Sweeping apps/desktop/src/main for show() / focus() / showInactive() / restore() outside window-reveal.ts leaves nothing ungated on a passing run: runtime-host-boot.ts:947 and :1400 go through the presentation and the controller, startup-presentation.ts:107 is the progress window's own focus() method (which now routes), startup-progress-window.ts:267 is a DOM button, and notifications-ipc-main.ts uses an OS Notification plus the gated controller. The browser-message-box disposition is where I disagree slightly with the wording, and that is the inline P3 — the code is unreachable on a passing run, not unreachable.

In hidden, the handoff window is still created, and never shown. createDesktopHostHandoffSurface builds it unconditionally and now passes revealMode: startupRevealMode(); both reveal sites in startup-progress-window.ts — the did-finish-load branch and setHandoff's needsAttention branch — go through focusWindow / showWindowInactive, which return immediately under hidden. So the window exists and loads, and nothing about it reaches the screen. Worth being explicit that this means a handoff prompt in a hidden run cannot be answered by a human; in an automated run that is the intent, but it is a real property rather than a no-op.

In inactive, one thing does run after the reveal, and it is not a window focus. detach calls focusWindow(target, deps.revealMode) and then focusComposer(). That helper ends at view.webContents.focus() — web-contents focus, not BrowserWindow.focus() — and it early-returns when !parent.isVisible(), so under hidden it never runs at all. Under inactive it does run against a window revealed by showInactive(). Whether webContents.focus() can raise a window to the foreground on any given platform is something I did not verify, so I am recording it as the one place a later reveal could in principle re-enter, not as a defect.

Production behaviour is very nearly unchanged, and the exceptions are worth naming rather than rounding off. resolveWindowRevealMode returns active whenever isPackaged || !isE2eRun, so shipped builds never see the other two modes. Within active I found three differences from the code that was replaced, all in the safe direction:

• focusWindow adds an isDestroyed() guard that the inline WorkHub calls in detach and navigateMain did not have.
• focusWindow adds isMinimized() / restore() on the startup did-finish-load attention branch, which previously did only show(); focus().
• showWindowInactive inherits showWindowOnceReady's if (win.isVisible()) return, where the old WorkHub progress card called floating?.showInactive() unconditionally on every progress-ready.

None is a regression. But "production 行为不变" is slightly stronger than what the diff does.

The three new tests are ablation-verified — I broke each mechanism and watched exactly one test fail. Removing the mode handling from focusWindow fails an automated run never lets a handoff pull the app to the front and summoning and docking honor the run reveal mode; dropping the hidden clamp from showWindowInactive fails the progress card stays hidden in a hidden run and inactive everywhere else. All three drive the real factories with fake windows that count show / showInactive / focus, so they exercise the production seam rather than the helper in isolation.

A trap for anyone repeating that ablation: the two suites do not consume the same artifact. workhub-presentation.test.ts rebuilds workhub-presentation.ts from src/ with esbuild, while startup-progress-window.test.ts runs the compiled output. My first mutation edited dist/main/window-reveal.js only, and the WorkHub suite stayed green — which for a few minutes looked like a test that names an invariant it does not pin. It was my mutation missing its target. Editing src/main/window-reveal.ts fails it immediately.

On the ~1 second of foreground occupancy (2 of 60 samples): I judge it outside this PR. Under hidden no window is revealed at all, so no window-reveal path can account for foreground time — whatever takes the foreground is the application, not a window. The lever for that already exists and is already keyed off the same mode: installDesktopStartupBranding(revealMode) calls app.dock.hide(), and dock-presentation.ts documents hide as "run as an accessory app … it never becomes frontmost, so a capture or E2E run cannot steal focus". But that call sits inside showDesktopStartupProgress, which runs after app.whenReady(). The interval between electron.launch() and that point is unguarded, and a sub-second activation there matches the observation. So this is pre-ready application activation, and closing it means making the accessory decision earlier than whenReady — or at the bundle/launch level — which is a different change from routing window reveals. I did not measure this myself; the reasoning is from the call ordering, and I would not treat it as located until someone confirms the blip disappears when the policy is applied earlier.

Scope and limits. Static review of all seven files plus window-reveal.ts, dock-presentation.ts, dev-single-instance.ts and the browser-message-box callers; a local apps/desktop main build used to run and mutate the two affected suites. The build reports many pre-existing type errors in my environment, which did not prevent emit and do not touch these files. I did not run Playwright, did not launch Electron, and did not reproduce the foreground sampling — so the webContents.focus() and the one-second questions are reasoned from source and call ordering, not observed. Single review line.

简体中文

绑定 bb067b55ce473393052dc718d1b943017cdbaecf,行内一条 P3,无 P0–P2。两项托管检查在此 exact head 终态为绿。

先说一个合并就绪性事实,因为派单里没提到:GitHub 报告本 PR 相对基线为 CONFLICTING。 CI 绿并不覆盖这一点。评审无需处理,提出来是免得之后意外。

e2e 可达的 reveal 都已入 gate,只有一处边界需要精确表述。 在 apps/desktop/src/main 全量搜 show() / focus() / showInactive() / restore()(排除 window-reveal.ts),通过的运行上没有遗漏:runtime-host-boot.ts:947/:1400 走 presentation 与 controller;startup-presentation.ts:107 是进度窗口自身的 focus() 方法(现已路由);startup-progress-window.ts:267 是 DOM 按钮;notifications-ipc-main.ts 用的是系统 Notification 加已入 gate 的 controller。我与措辞略有分歧的是 browser-message-box,即行内那条 P3 —— 它在"通过的运行"上不可达,而非不可达。

hidden 下 handoff 窗口仍会被创建,且从不显示。 两处 reveal 点都走 focusWindow / showWindowInactive,在 hidden 下立即返回。值得写明其含义:hidden 运行中的 handoff 提示无法由人应答 —— 自动化运行中这是本意,但它是一个真实性质,不是空操作。

inactive 下确实有一处在 reveal 之后运行,但它不是窗口 focus。 detach 在 focusWindow 之后调用 focusComposer(),后者最终是 view.webContents.focus()(web contents 而非 BrowserWindow.focus()),且在 !parent.isVisible() 时提前返回,故 hidden 下根本不执行。webContents.focus() 在各平台会不会把窗口提到前台,我没有验证 —— 因此我把它记为"原则上可能再次进入的唯一位置",而不是缺陷。

生产行为极接近不变,但例外值得点名而非四舍五入。 resolveWindowRevealMode 在 isPackaged || !isE2eRun 时一律返回 active,故发布版永远看不到另两种模式。在 active 内我找到三处与被替换代码的差异,方向都是安全的:focusWindow 增加了 WorkHub 内联调用原本没有的 isDestroyed() 守卫;focusWindow 在启动 did-finish-load 的 attention 分支上增加了 isMinimized() / restore();showWindowInactive 继承了 showWindowOnceReady 的 isVisible() 提前返回,而旧的 WorkHub 进度卡是每次 progress-ready 都无条件 showInactive()。都不是回归,但"production 行为不变"比 diff 实际所做的略强。

三条新增测试均经消融验证 —— 我逐一破坏机制,每次恰好打红一条。 去掉 focusWindow 的 mode 处理 → 打红 handoff 那条与 summoning 那条;去掉 showWindowInactive 的 hidden 钳制 → 打红进度卡那条。三条都以假窗口计数 show/showInactive/focus 驱动真实工厂,走的是生产 seam。

给重复该消融的人一个陷阱提示:两个套件消费的产物不同。 workhub-presentation.test.ts 用 esbuild 从 src/ 重新编译,而 startup-progress-window.test.ts 跑的是已编译产物。我第一次只改了 dist/main/window-reveal.js,WorkHub 套件全绿 —— 一度看起来像"测试声称了它并未钉住的不变量"。那是我的变异没打中目标。 改 src/main/window-reveal.ts 后立即变红。

关于约 1 秒的前台占用(60 个样本中 2 个):我判断不在本 PR 范围内。 hidden 下根本没有窗口被 reveal,因此没有任何窗口路径能解释这段前台时间 —— 占据前台的是应用,不是窗口。对应的手段已经存在且同样以 mode 为键:installDesktopStartupBranding(revealMode) 会调用 app.dock.hide(),而 dock-presentation.ts 对 hide 的文档正是"以附属应用运行……永不成为最前端,因此捕获或 E2E 运行不会抢走开发者焦点"。但该调用位于 showDesktopStartupProgress 内,即 app.whenReady() 之后。electron.launch() 到该点之间是无防护的,而不到一秒的激活正好落在这一区间。 所以这是 ready 之前的应用级激活,不是窗口 reveal;要消除它,需要把附属策略的决定提前到 whenReady 之前(或放到 bundle/启动层),那是与"路由窗口 reveal"不同的一项改动。我没有自己测量;以上是基于调用顺序的推理,在有人确认"策略提前后该现象消失"之前,我不会认为它已被定位。

本次审查的边界:七个文件加 window-reveal.ts、dock-presentation.ts、dev-single-instance.ts 与 browser-message-box 调用方的静态审查;本地构建 apps/desktop main 以运行并消融两个受影响套件(构建在我的环境下报大量既有类型错误,不影响产出,也不涉及这些文件)。未运行 Playwright、未启动 Electron、未复现前台采样 —— 因此 webContents.focus() 与那 1 秒的问题是从源码与调用顺序推理而来,并非观测所得。单线。

Automated review notice: This comment was posted by an automated review agent operated by jackwener (seat: kabi-opus). It is not an independent human review and does not replace one.

[jackwener]

[P3] Two call sites still bypass this authority, and the "e2e-unreachable" disposition that excuses them holds only for a passing run.

(Anchored here because browser-message-box.ts is not in this diff, so GitHub cannot take a comment on it. This is the function those sites would use.)

browser-message-box.ts:226-227 does a raw win.show(); win.focus();. showBrowserMessageBox reaches it from two places in main.ts, and neither is a happy-path call:

• main.ts:116 — the dev single-instance loser dialog, gated by shouldShowLoserDialog(process.argv). That predicate is !argv.includes(DEV_CONFLICT_HANDLED_BY_LAUNCHER_FLAG) and nothing else: it does not consult isIsolatedE2e, MAKA_E2E_FIXTURE, or the reveal mode. A fixture launching while a stray Electron from an earlier spec is still alive lands here.
• main.ts:252 — the fatal-startup diagnostic box, inside finally { … app.exit(1) }.

So the symptom this PR exists to remove — Electron taking the foreground while someone runs e2e locally — survives on the path where a run is already going wrong. That is also the moment it is most disruptive: the developer is watching a failing spec and the box pulls focus.

I am filing this as P3, not higher, for two reasons. It cannot be reached by a passing run, so it is outside the stated scope; and gating a crash dialog is a real product decision rather than an oversight — on a packaged build you want that box. The narrow point is that the disposition table's justification is "e2e unreachable", and the accurate statement is "unreachable on a passing e2e run".

Minimal fix, if you want it in scope: route these two through the same authority the rest of the PR uses — focusWindow(win, mode) with the startup reveal mode — which already no-ops under hidden and reveals without activating under inactive. If you would rather leave it, the cheaper change is to say so in the disposition table, so the next person auditing this does not have to re-derive the callers.

Same reasoning, not filed separately: cursor-overlay-window.ts:388, pip-window.ts:622 and permission-overlay-controller.ts:217 still call showInactive() outside the gate. No current e2e spec reaches computer-use or the permission overlay, so they are genuinely unreachable today, and showInactive() does not steal focus in any case — but that judgment is a property of the current spec set, not of the code.

Automated review notice: This comment was posted by an automated review agent operated by jackwener (seat: kabi-opus). It is not an independent human review and does not replace one.

[Astro-Han]

Fixed in 67ce2ad. presentBrowserMessageBox now reveals through focusWindow(win, appearance.revealMode) instead of the raw show() + focus(); main.ts passes startupRevealMode() for both the dev single-instance loser dialog and the fatal-startup diagnostic box, and runtime-host-boot.ts passes the mode it already resolved. Under hidden the box is created and loaded but never revealed, and since nobody can answer an invisible dialog it settles as its cancel id rather than leaving the caller pending — otherwise the loser instance would never reach app.exit. Production is active, so the only difference there is the isDestroyed()/isMinimized() guards focusWindow adds. I left the showInactive() calls in cursor-overlay-window.ts, pip-window.ts and permission-overlay-controller.ts outside the gate: no current spec reaches them and an inactive reveal cannot take the foreground, which is the property this PR defends — they are now listed in the PR body's disposition table with that reasoning.