From e19aac5b20860ecdcd8203aca03465caef6ffc63 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Thu, 3 Sep 2026 00:21:28 +0800 Subject: [PATCH 1/2] fix(desktop): keep e2e windows visible without stealing focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MAKA_E2E_SHOW_WINDOW` conflated two things: whether the window appears and whether the app takes the foreground. Every fixture that sets it — and `e2e/fixtures.ts` sets it for eight of them, plus every run on a CI Linux display — got a dock tile, an activated app, and a window that jumped in front of whatever the developer was doing, repeatedly, for the length of the suite. Nothing in those fixtures wants focus: they want the compositor (a hidden window is throttled to ~1fps under xvfb) and a real layout for geometry assertions. So the flag now lifts the window's visibility and stops there. The single `startHidden` boolean becomes the run's reveal mode — `hidden`, `inactive`, `active` — resolved once in `window-reveal.ts` and consumed by both the reveal gate and the dock rule: - `hidden`: unchanged. E2E captures paint the hidden window. - `inactive`: reveal with `showInactive()`, stay an accessory app, and answer a focus request with a reveal and nothing more. `maximize()` reveals a hidden window and that reveal activates, so an inactive reveal comes first and leaves it nothing to show. - `active`: the product, unchanged. The E2E harness had the same conflation of its own: the prompt-rail worker window is re-revealed before every test in the file with `window.show()`, which activates the app each time no matter what the main process decided. It reveals inactively now. Verified on macOS by sampling the frontmost application while running `playwright test e2e/prompt-rail.spec.ts`: before, the Electron app held the foreground in 14 of 130 samples; after, in 0 of 130, with all nine tests still passing. Generated-by: Claude Code --- apps/desktop/e2e/fixtures.ts | 6 +- .../main/__tests__/window-reveal-mode.test.ts | 174 ++++++++++++++++++ .../src/main/desktop-shell-presentation.ts | 5 +- apps/desktop/src/main/dock-presentation.ts | 16 +- apps/desktop/src/main/main-window.ts | 23 +-- apps/desktop/src/main/runtime-host-boot.ts | 12 +- apps/desktop/src/main/window-reveal.ts | 63 +++++-- scripts/desktop-real-window-smoke.mjs | 8 +- 8 files changed, 269 insertions(+), 38 deletions(-) create mode 100644 apps/desktop/src/main/__tests__/window-reveal-mode.test.ts diff --git a/apps/desktop/e2e/fixtures.ts b/apps/desktop/e2e/fixtures.ts index 051621a964..8335b23ade 100644 --- a/apps/desktop/e2e/fixtures.ts +++ b/apps/desktop/e2e/fixtures.ts @@ -525,7 +525,11 @@ async function setPromptRailWindowVisible( await worker.app.evaluate(({ BrowserWindow }, shouldShow) => { const window = BrowserWindow.getAllWindows()[0]; if (!window) throw new Error('the prompt-rail BrowserWindow is missing'); - if (shouldShow) window.show(); + // showInactive, not show: this worker window is re-revealed between every + // test in the file, and show() activates the app each time — a suite run + // would yank the developer's foreground away a dozen times over. The + // window still needs to be on screen for the compositor. + if (shouldShow) window.showInactive(); else window.hide(); }, visible); } diff --git a/apps/desktop/src/main/__tests__/window-reveal-mode.test.ts b/apps/desktop/src/main/__tests__/window-reveal-mode.test.ts new file mode 100644 index 0000000000..9b0c17fe27 --- /dev/null +++ b/apps/desktop/src/main/__tests__/window-reveal-mode.test.ts @@ -0,0 +1,174 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { resolveDockPresentation } from '../dock-presentation.js'; +import { + createWindowRevealGate, + resolveWindowRevealMode, + showWindowOnceReady, + type FocusableRevealableWindow, +} from '../window-reveal.js'; + +/** + * Fake BrowserWindow that records every call the reveal gate makes, so + * "revealed but never activated" can be asserted without an Electron runtime. + */ +function fakeWindow(): FocusableRevealableWindow & { + calls: string[]; + visible: boolean; + destroyed: boolean; + minimized: boolean; +} { + const win = { + calls: [] as string[], + visible: false, + destroyed: false, + minimized: false, + isDestroyed: () => win.destroyed, + isVisible: () => win.visible, + isMinimized: () => win.minimized, + show() { + win.calls.push('show'); + win.visible = true; + }, + showInactive() { + win.calls.push('showInactive'); + win.visible = true; + }, + restore() { + win.calls.push('restore'); + win.minimized = false; + }, + focus() { + win.calls.push('focus'); + }, + maximize() { + win.calls.push('maximize'); + win.visible = true; + }, + }; + return win; +} + +describe('resolveWindowRevealMode', () => { + it('leaves a product run active', () => { + assert.equal(resolveWindowRevealMode(false, false), 'active'); + // A stray MAKA_E2E_SHOW_WINDOW outside an E2E run changes nothing. + assert.equal(resolveWindowRevealMode(false, true), 'active'); + }); + + it('hides an E2E run that did not ask for a window', () => { + assert.equal(resolveWindowRevealMode(true, false), 'hidden'); + }); + + it('gives an E2E run that asked for a window pixels, not focus', () => { + assert.equal(resolveWindowRevealMode(true, true), 'inactive'); + }); +}); + +describe('resolveDockPresentation', () => { + it('has no dock off macOS', () => { + for (const mode of ['hidden', 'inactive', 'active'] as const) { + assert.equal(resolveDockPresentation('win32', mode), 'none'); + assert.equal(resolveDockPresentation('linux', mode), 'none'); + } + }); + + it('shows the brand mark only for a product run', () => { + assert.equal(resolveDockPresentation('darwin', 'active'), 'icon'); + }); + + it('stays an accessory app for every E2E run, visible window included', () => { + assert.equal(resolveDockPresentation('darwin', 'hidden'), 'hide'); + assert.equal(resolveDockPresentation('darwin', 'inactive'), 'hide'); + }); +}); + +describe('showWindowOnceReady', () => { + it('reveals an active run with show()', () => { + const win = fakeWindow(); + showWindowOnceReady(win, 'active'); + assert.deepEqual(win.calls, ['show']); + }); + + it('reveals an inactive run without activating the app', () => { + const win = fakeWindow(); + showWindowOnceReady(win, 'inactive'); + assert.deepEqual(win.calls, ['showInactive']); + }); + + it('never reveals a hidden run', () => { + const win = fakeWindow(); + showWindowOnceReady(win, 'hidden'); + assert.deepEqual(win.calls, []); + }); + + it('ignores a destroyed or already visible window', () => { + const destroyed = fakeWindow(); + destroyed.destroyed = true; + showWindowOnceReady(destroyed, 'inactive'); + assert.deepEqual(destroyed.calls, []); + + const shown = fakeWindow(); + shown.visible = true; + showWindowOnceReady(shown, 'inactive'); + assert.deepEqual(shown.calls, []); + }); +}); + +describe('createWindowRevealGate', () => { + it('flushes a deferred focus request as a real activation for a product run', () => { + const gate = createWindowRevealGate('active'); + const win = fakeWindow(); + gate.requestFocus(win); + assert.deepEqual(win.calls, []); + gate.markReady(win); + assert.deepEqual(win.calls, ['show', 'show', 'focus']); + }); + + it('answers a focus request on an inactive run with a reveal and nothing more', () => { + const gate = createWindowRevealGate('inactive'); + const win = fakeWindow(); + gate.requestFocus(win); + gate.markReady(win); + assert.deepEqual(win.calls, ['showInactive']); + // A focus request after readiness must not raise the app either. + gate.requestFocus(win); + assert.deepEqual(win.calls, ['showInactive']); + }); + + it('reveals inactively before maximizing, so the maximize cannot raise the app', () => { + const gate = createWindowRevealGate('inactive'); + const win = fakeWindow(); + gate.requestMaximize(win); + gate.markReady(win); + assert.deepEqual(win.calls, ['showInactive', 'maximize']); + }); + + it('keeps a hidden run hidden on every path', () => { + const gate = createWindowRevealGate('hidden'); + const win = fakeWindow(); + gate.requestFocus(win); + gate.requestMaximize(win); + gate.markReady(win); + assert.deepEqual(win.calls, []); + }); +}); diff --git a/apps/desktop/src/main/desktop-shell-presentation.ts b/apps/desktop/src/main/desktop-shell-presentation.ts index 2a6d96daa5..754572bfa6 100644 --- a/apps/desktop/src/main/desktop-shell-presentation.ts +++ b/apps/desktop/src/main/desktop-shell-presentation.ts @@ -22,9 +22,10 @@ import { applyAppIcon } from './app-icon-surface.js'; import { installApplicationMenu } from './application-menu.js'; import { resolveDockPresentation } from './dock-presentation.js'; import type { createMainWindowController } from './main-window.js'; +import type { WindowRevealMode } from './window-reveal.js'; interface DesktopShellPresentationDeps { - readonly startHidden: boolean; + readonly revealMode: WindowRevealMode; readonly mainWindowController: ReturnType; readonly focusOrCreateWindow: () => void; readonly onIconError: (error: unknown) => void; @@ -36,7 +37,7 @@ export function installDesktopShellPresentation( ): void { const dockPresentation = resolveDockPresentation( process.platform, - deps.startHidden, + deps.revealMode, ); if (app.dock) { if (dockPresentation === 'hide') { diff --git a/apps/desktop/src/main/dock-presentation.ts b/apps/desktop/src/main/dock-presentation.ts index 55e7d34107..4ec65bb497 100644 --- a/apps/desktop/src/main/dock-presentation.ts +++ b/apps/desktop/src/main/dock-presentation.ts @@ -17,15 +17,21 @@ * under the License. */ +import type { WindowRevealMode } from './window-reveal.js'; + /** * What the macOS dock should show for this run. * * Its own module, free of an `electron` import, so the rule can be tested * without launching Electron. The branch this replaced lived inside * `app.whenReady()` and keyed off a re-derivation of the start-hidden - * condition that had already drifted from the real one — it missed the - * `MAKA_E2E_SHOW_WINDOW` escape hatch, so a window a developer explicitly - * asked to see still launched as an accessory app. + * condition that had already drifted from the real one. + * + * Keyed off the run's reveal mode, not off "does a window appear": an E2E run + * that asks for a visible window (`MAKA_E2E_SHOW_WINDOW`) wants pixels, not + * the foreground — the compositor throttles a hidden window and geometry + * assertions need a real layout, and neither needs a dock tile. Only a real + * `active` run gets one. * * - `hide`: run as an accessory app. No dock tile, no dock bounce, and it * never becomes frontmost, so a capture or E2E run cannot steal focus from @@ -37,8 +43,8 @@ */ export function resolveDockPresentation( platform: NodeJS.Platform, - startHidden: boolean, + revealMode: WindowRevealMode, ): 'hide' | 'icon' | 'none' { if (platform !== 'darwin') return 'none'; - return startHidden ? 'hide' : 'icon'; + return revealMode === 'active' ? 'icon' : 'hide'; } diff --git a/apps/desktop/src/main/main-window.ts b/apps/desktop/src/main/main-window.ts index a2c61a5473..aeb083bf1b 100644 --- a/apps/desktop/src/main/main-window.ts +++ b/apps/desktop/src/main/main-window.ts @@ -35,7 +35,7 @@ import { reloadMainRendererProcess, } from './main-renderer-process-gone.js'; import { isDarkAppearance, isThemePreference, toNativeThemeSource } from './theme-source.js'; -import { createWindowRevealGate } from './window-reveal.js'; +import { createWindowRevealGate, type WindowRevealMode } from './window-reveal.js'; import { createWindowsMaximizeRendererSync } from './windows-maximize-renderer-sync.js'; import { parseDesktopSessionResourceKey, @@ -97,7 +97,7 @@ interface MainWindowControllerDeps { settingsStore: SettingsReader; // main.ts computes this from the same isE2e gate that also guards userData // and the fake backend, so main-window.ts owns no env policy of its own. - startHidden: boolean; + revealMode: WindowRevealMode; onClose?: () => void; onRendererProcessGone: (details: Electron.RenderProcessGoneDetails) => void | Promise; } @@ -165,22 +165,23 @@ const titleBarOverlayOptions = ( }); export function createMainWindowController(deps: MainWindowControllerDeps): MainWindowController { - const { workspaceRoot, e2eFixture, settingsStore, startHidden } = deps; + const { workspaceRoot, e2eFixture, settingsStore } = deps; const liveBrowserScopes = new Map(); - // PR-SHOW-AFTER-FIRST-COMMIT: windows launched hidden (startHidden covers + // PR-SHOW-AFTER-FIRST-COMMIT: windows launched hidden (`hidden` covers // e2e-fixture capture and E2E — see main.ts) must never be revealed; // e2e-fixture captures run on the hidden window and E2E drives it headless. - // `!app.isPackaged` mirrors the original creation-time gate so a packaged - // build ignores a stray startHidden flag. The fallback timer, the - // renderer-ready IPC, and focus() all route their show() through this - // predicate via the reveal gate below. - const keepHiddenForE2eFixture = !app.isPackaged && startHidden; + // A run that asked for a visible window is `inactive`: it reveals, but never + // activates the app. `!app.isPackaged` mirrors the original creation-time + // gate so a packaged build ignores a stray E2E flag. The fallback timer, the + // renderer-ready IPC, and focus() all route their show() through this mode + // via the reveal gate below. + const revealMode: WindowRevealMode = app.isPackaged ? 'active' : deps.revealMode; // ChatGPT Pro review P2: focus() (second-instance / activate) used to call // mainWindow.show() directly, bypassing the reveal gate — re-launching or // clicking the dock icon during the pre-commit window would flash the // skeleton anyway. The gate defers those focus requests until markReady. - const revealGate = createWindowRevealGate(keepHiddenForE2eFixture); + const revealGate = createWindowRevealGate(revealMode); let showFallbackTimer: NodeJS.Timeout | undefined; let rendererRecoveryReadiness: | { @@ -197,7 +198,7 @@ export function createMainWindowController(deps: MainWindowControllerDeps): Main }; const armShowFallbackTimer = (target: BrowserWindow): void => { clearShowFallbackTimer(); - if (keepHiddenForE2eFixture || target.isDestroyed() || target.isVisible()) return; + if (revealMode === 'hidden' || target.isDestroyed() || target.isVisible()) return; showFallbackTimer = setTimeout(() => { showFallbackTimer = undefined; if (!target.isDestroyed()) revealGate.markReady(target); diff --git a/apps/desktop/src/main/runtime-host-boot.ts b/apps/desktop/src/main/runtime-host-boot.ts index f19f38ded0..a51af88dd2 100644 --- a/apps/desktop/src/main/runtime-host-boot.ts +++ b/apps/desktop/src/main/runtime-host-boot.ts @@ -118,6 +118,7 @@ import { type ReconnectableReadIpcMain, } from "./ipc-reconnect-policy.js"; import { createMainWindowController } from "./main-window.js"; +import { resolveWindowRevealMode } from "./window-reveal.js"; import type { DesktopRuntimeHostIdentity } from "../preload/bridge-contract.js"; import { captureDesktopDiagnosticEnvironment, @@ -472,15 +473,16 @@ function ensureMcpReady(): Promise { return mcpStartup; } const keepSystemAwake = createKeepSystemAwakeController(powerSaveBlocker); -const startHidden = - (Boolean(e2eFixture) || isIsolatedE2e) && - process.env.MAKA_E2E_SHOW_WINDOW !== "1"; +const revealMode = resolveWindowRevealMode( + Boolean(e2eFixture) || isIsolatedE2e, + process.env.MAKA_E2E_SHOW_WINDOW === "1", +); let onMainWindowClose = (): void => {}; const mainWindowController = createMainWindowController({ workspaceRoot, e2eFixture, settingsStore, - startHidden, + revealMode, onClose: () => onMainWindowClose(), onRendererProcessGone: async (details) => { const diagnosticInput = createDesktopMainRendererDiagnosticInput({ @@ -1896,7 +1898,7 @@ function wireLifecycle(): void { resumeQuit: () => app.quit(), }); installDesktopShellPresentation({ - startHidden, + revealMode, mainWindowController, focusOrCreateWindow: quitCoordinator.focusOrCreateWindow, onIconError: (error) => diff --git a/apps/desktop/src/main/window-reveal.ts b/apps/desktop/src/main/window-reveal.ts index a28827cb98..c2bb66dfdf 100644 --- a/apps/desktop/src/main/window-reveal.ts +++ b/apps/desktop/src/main/window-reveal.ts @@ -29,27 +29,58 @@ * can't be imported under plain `node --test` because it pulls in `electron`). */ +/** + * How far this run is allowed to go when it reveals the main window. + * + * - `hidden`: never reveal. E2E-fixture captures paint the hidden window via + * `paintWhenInitiallyHidden`, so pixels arrive without a window on screen. + * - `inactive`: reveal, never raise. An E2E run that asked for a visible + * window (`MAKA_E2E_SHOW_WINDOW`) needs the compositor and a real layout — + * a hidden window is throttled to ~1fps under xvfb and geometry assertions + * need one — but taking the foreground from the developer running the suite + * is never part of that. + * - `active`: the product. Reveal and honor foreground intent. + */ +export type WindowRevealMode = 'hidden' | 'inactive' | 'active'; + +/** + * The run's reveal mode. `showWindowRequested` (MAKA_E2E_SHOW_WINDOW) asks for + * a visible window; it does not ask for focus, so it can only lift `hidden` to + * `inactive`. Only a run that is not an E2E run at all is `active`. + */ +export function resolveWindowRevealMode( + isE2eRun: boolean, + showWindowRequested: boolean, +): WindowRevealMode { + if (!isE2eRun) return 'active'; + return showWindowRequested ? 'inactive' : 'hidden'; +} + /** Minimal structural view of the BrowserWindow surface the gate touches. */ export interface RevealableWindow { isDestroyed(): boolean; isVisible(): boolean; show(): void; + /** Reveals without activating the app — the `inactive` mode's only reveal. */ + showInactive(): void; } /** * Reveal `win` unless it must stay hidden. Idempotent and focus-safe: - * - `keepHidden` true (e2e-fixture capture): never reveal — capture runs on - * the hidden window via `paintWhenInitiallyHidden`. + * - `hidden` mode: never reveal. + * - `inactive` mode: reveal with showInactive(), so the window appears where + * it belongs without pulling the app to the front. * - null / destroyed window: no-op (teardown raced the timer or the IPC). * - already visible: no-op, so a second signal (HMR reload re-fires * notifyRendererReady, or the timer races the signal) never re-shows and * never steals foreground focus. */ -export function showWindowOnceReady(win: RevealableWindow | null, keepHidden: boolean): void { - if (keepHidden) return; +export function showWindowOnceReady(win: RevealableWindow | null, mode: WindowRevealMode): void { + if (mode === 'hidden') return; if (!win || win.isDestroyed()) return; if (win.isVisible()) return; - win.show(); + if (mode === 'inactive') win.showInactive(); + else win.show(); } /** Focus surface for deferred focus requests (see createWindowRevealGate). */ @@ -86,25 +117,35 @@ export interface WindowRevealGate { * intent and markReady applies it right before the reveal, so the window's * first on-screen frame is already maximized. * - * `keepHidden` windows (e2e-fixture capture / E2E) never show, maximize, or - * take focus from any path — captures run while the developer works elsewhere. + * `hidden` windows (e2e-fixture capture / E2E) never show, maximize, or take + * focus from any path — captures run while the developer works elsewhere. + * `inactive` windows appear but stay behind: a focus request reveals them and + * stops there, so an E2E run's own activate / second-instance traffic cannot + * pull the app in front of whatever the developer is doing. */ -export function createWindowRevealGate(keepHidden: boolean): WindowRevealGate { +export function createWindowRevealGate(mode: WindowRevealMode): WindowRevealGate { let ready = false; let pendingFocus = false; let pendingMaximize = false; const focusNow = (win: FocusableRevealableWindow | null): void => { - if (keepHidden) return; + if (mode === 'hidden') return; if (!win || win.isDestroyed()) return; + if (mode === 'inactive') { + showWindowOnceReady(win, mode); + return; + } if (win.isMinimized()) win.restore(); win.show(); win.focus(); }; const maximizeNow = (win: FocusableRevealableWindow | null): void => { - if (keepHidden) return; + if (mode === 'hidden') return; if (!win || win.isDestroyed()) return; + // maximize() reveals a still-hidden window, and that reveal activates the + // app. Reveal it inactively first so the maximize has nothing left to show. + if (mode === 'inactive') showWindowOnceReady(win, mode); win.maximize(); }; @@ -122,7 +163,7 @@ export function createWindowRevealGate(keepHidden: boolean): WindowRevealGate { pendingMaximize = false; maximizeNow(win); } - showWindowOnceReady(win, keepHidden); + showWindowOnceReady(win, mode); if (pendingFocus) { pendingFocus = false; focusNow(win); diff --git a/scripts/desktop-real-window-smoke.mjs b/scripts/desktop-real-window-smoke.mjs index 53c10a6b3b..a2566a50ae 100644 --- a/scripts/desktop-real-window-smoke.mjs +++ b/scripts/desktop-real-window-smoke.mjs @@ -303,9 +303,11 @@ async function launchElectron(args, diagnostics) { // a developer with `npm run dev` open smoked the dev server instead of the // build this script just made (VITE_DEV_SERVER_URL), and the run touched // the real $HOME. A fixture window also starts hidden for its whole - // lifecycle (`startHidden` in `main.ts`), which leaves this gate with - // nothing to look at — showWindow opts this run back into a visible window, - // and the dock rule follows it. + // lifecycle (the `hidden` reveal mode — see `window-reveal.ts`), which + // leaves this gate with nothing to look at. showWindow opts this run into a + // visible window; it stays an accessory app that never takes the foreground, + // so the window appears without interrupting whoever launched it. Clicking + // it still brings it forward when someone wants to drive it by hand. const env = buildFixtureEnv(userDataDir, homeDir, { scenario: args.startupOnly ? undefined : args.scenario, showWindow: true, From 926275f69679c3cf1e52b0c4075502cec5293765 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Thu, 3 Sep 2026 11:17:34 +0800 Subject: [PATCH 2/2] fix(desktop): keep an inactive reveal working on a native Wayland session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `showInactive()` is unsupported when Electron runs on native Wayland, so a window revealed that way can simply fail to appear — which brings back the ~1fps compositor throttling and the geometry failures that asking for a visible window exists to avoid. The repo already knew this and already carried the remedy, but it lived inside one launcher and only that launcher used it, so routing two more launch paths into inactive reveals walked straight past it. Move the rule next to `isCiLinuxDisplay` in `fixture-env.mjs`, which is already the shared authority for what a fixture launch environment looks like, and have it return only the extra arguments so each launcher composes it with its own. All three inactive launchers now go through it. Hosted CI is unaffected either way: it runs on an Xvfb X11 display, where showInactive() is supported. Two corrections from the same review: The reveal gate's comment claimed the deferred maximize makes the window's first on-screen frame the maximized one. That is only true in `active` mode; an `inactive` window is revealed first and maximized second, because the reveal maximize() performs is an activating one. Say what the code does, and why the visible zoom is the cheaper half of that trade. `resolveWindowRevealMode` was described as the single authority for how far a run may go, but the packaged-build override sat in `main-window.ts` where only the reveal gate could see it — the dock rule read the un-overridden value, so a packaged build carrying a stray E2E variable got a window that may take focus and a dock that hides its tile. Fold `isPackaged` into the resolver and delete the module-local override, so both consumers read one answer. Generated-by: Claude Code --- apps/desktop/e2e/fixtures.ts | 17 ++++-- .../main/__tests__/window-reveal-mode.test.ts | 19 +++++-- apps/desktop/src/main/main-window.ts | 8 +-- apps/desktop/src/main/runtime-host-boot.ts | 1 + apps/desktop/src/main/window-reveal.ts | 21 +++++-- scripts/desktop-real-window-smoke.mjs | 6 +- scripts/fixture-env.mjs | 23 ++++++++ scripts/fixture-env.test.mjs | 56 +++++++++++++++++++ scripts/fixture-window.mjs | 19 +------ 9 files changed, 133 insertions(+), 37 deletions(-) create mode 100644 scripts/fixture-env.test.mjs diff --git a/apps/desktop/e2e/fixtures.ts b/apps/desktop/e2e/fixtures.ts index 8335b23ade..02130965c3 100644 --- a/apps/desktop/e2e/fixtures.ts +++ b/apps/desktop/e2e/fixtures.ts @@ -32,7 +32,11 @@ import { tryAcquireInteractiveRootOwner, } from '@maka/storage/root-authority'; import { openInteractiveRuntimePolicyStoresForWrite } from '@maka/storage/runtime-policy-stores'; -import { buildFixtureEnv, isCiLinuxDisplay } from '../../../scripts/fixture-env.mjs'; +import { + buildFixtureEnv, + inactiveWindowPlatformArgs, + isCiLinuxDisplay, +} from '../../../scripts/fixture-env.mjs'; import { closeElectronApplication } from '../../../scripts/electron-lifecycle.mjs'; const DESKTOP_ROOT = process.cwd(); @@ -457,17 +461,20 @@ async function withE2eWindow( // Legacy E2E specs assert Chinese labels and should not inherit the CI // host locale. E2e-fixture workspaces use the explicit renderer override. if (locale && !e2eFixtureScenario) await seedE2eLocale(userDataDir, locale); + // xvfb throttles a hidden window's compositor to ~1fps. Geometry fixtures + // opt in locally; every fixture is visible on isolated CI X. + const visibleWindow = showWindow || isCiLinuxDisplay(); app = await electron.launch({ - args: ['.'], + // A visible fixture window is revealed inactively, which needs XWayland + // on a native Wayland session. + args: ['.', ...(visibleWindow ? inactiveWindowPlatformArgs() : [])], cwd: DESKTOP_ROOT, env: buildFixtureEnv(userDataDir, homeDir, { scenario: e2eFixtureScenario, locale, platform, scrollMotion, - // xvfb throttles a hidden window's compositor to ~1fps. Geometry - // fixtures opt in locally; every fixture is visible on isolated CI X. - showWindow: showWindow || isCiLinuxDisplay(), + showWindow: visibleWindow, }), }); app.on('console', (message) => { diff --git a/apps/desktop/src/main/__tests__/window-reveal-mode.test.ts b/apps/desktop/src/main/__tests__/window-reveal-mode.test.ts index 9b0c17fe27..24eb06cd44 100644 --- a/apps/desktop/src/main/__tests__/window-reveal-mode.test.ts +++ b/apps/desktop/src/main/__tests__/window-reveal-mode.test.ts @@ -70,17 +70,28 @@ function fakeWindow(): FocusableRevealableWindow & { describe('resolveWindowRevealMode', () => { it('leaves a product run active', () => { - assert.equal(resolveWindowRevealMode(false, false), 'active'); + assert.equal(resolveWindowRevealMode(false, false, false), 'active'); // A stray MAKA_E2E_SHOW_WINDOW outside an E2E run changes nothing. - assert.equal(resolveWindowRevealMode(false, true), 'active'); + assert.equal(resolveWindowRevealMode(false, true, false), 'active'); }); it('hides an E2E run that did not ask for a window', () => { - assert.equal(resolveWindowRevealMode(true, false), 'hidden'); + assert.equal(resolveWindowRevealMode(true, false, false), 'hidden'); }); it('gives an E2E run that asked for a window pixels, not focus', () => { - assert.equal(resolveWindowRevealMode(true, true), 'inactive'); + assert.equal(resolveWindowRevealMode(true, true, false), 'inactive'); + }); + + it('ignores a stray E2E flag in a packaged build', () => { + // Both consumers read this one answer, so a packaged build cannot end up + // with a window that takes focus and a dock that hides its tile. + assert.equal(resolveWindowRevealMode(true, false, true), 'active'); + assert.equal(resolveWindowRevealMode(true, true, true), 'active'); + assert.equal( + resolveDockPresentation('darwin', resolveWindowRevealMode(true, true, true)), + 'icon', + ); }); }); diff --git a/apps/desktop/src/main/main-window.ts b/apps/desktop/src/main/main-window.ts index aeb083bf1b..9f8fbc1e84 100644 --- a/apps/desktop/src/main/main-window.ts +++ b/apps/desktop/src/main/main-window.ts @@ -172,11 +172,9 @@ export function createMainWindowController(deps: MainWindowControllerDeps): Main // e2e-fixture capture and E2E — see main.ts) must never be revealed; // e2e-fixture captures run on the hidden window and E2E drives it headless. // A run that asked for a visible window is `inactive`: it reveals, but never - // activates the app. `!app.isPackaged` mirrors the original creation-time - // gate so a packaged build ignores a stray E2E flag. The fallback timer, the - // renderer-ready IPC, and focus() all route their show() through this mode - // via the reveal gate below. - const revealMode: WindowRevealMode = app.isPackaged ? 'active' : deps.revealMode; + // activates the app. The fallback timer, the renderer-ready IPC, and focus() + // all route their show() through this mode via the reveal gate below. + const revealMode: WindowRevealMode = deps.revealMode; // ChatGPT Pro review P2: focus() (second-instance / activate) used to call // mainWindow.show() directly, bypassing the reveal gate — re-launching or // clicking the dock icon during the pre-commit window would flash the diff --git a/apps/desktop/src/main/runtime-host-boot.ts b/apps/desktop/src/main/runtime-host-boot.ts index a51af88dd2..a326187f48 100644 --- a/apps/desktop/src/main/runtime-host-boot.ts +++ b/apps/desktop/src/main/runtime-host-boot.ts @@ -476,6 +476,7 @@ const keepSystemAwake = createKeepSystemAwakeController(powerSaveBlocker); const revealMode = resolveWindowRevealMode( Boolean(e2eFixture) || isIsolatedE2e, process.env.MAKA_E2E_SHOW_WINDOW === "1", + app.isPackaged, ); let onMainWindowClose = (): void => {}; const mainWindowController = createMainWindowController({ diff --git a/apps/desktop/src/main/window-reveal.ts b/apps/desktop/src/main/window-reveal.ts index c2bb66dfdf..8be37cd444 100644 --- a/apps/desktop/src/main/window-reveal.ts +++ b/apps/desktop/src/main/window-reveal.ts @@ -47,12 +47,18 @@ export type WindowRevealMode = 'hidden' | 'inactive' | 'active'; * The run's reveal mode. `showWindowRequested` (MAKA_E2E_SHOW_WINDOW) asks for * a visible window; it does not ask for focus, so it can only lift `hidden` to * `inactive`. Only a run that is not an E2E run at all is `active`. + * + * A packaged build ignores a stray E2E flag entirely. That rule lives here + * rather than in one consumer, because both of them — the reveal gate and the + * dock rule — have to read the same answer; a build whose window may take + * focus while the dock treats it as an accessory app is neither mode. */ export function resolveWindowRevealMode( isE2eRun: boolean, showWindowRequested: boolean, + isPackaged: boolean, ): WindowRevealMode { - if (!isE2eRun) return 'active'; + if (isPackaged || !isE2eRun) return 'active'; return showWindowRequested ? 'inactive' : 'hidden'; } @@ -114,8 +120,12 @@ export interface WindowRevealGate { * The same deferral applies to restoring a saved maximized state: Electron's * BrowserWindow.maximize() reveals a still-hidden window (verified on macOS), * so createWindow must not call it directly — requestMaximize holds the - * intent and markReady applies it right before the reveal, so the window's - * first on-screen frame is already maximized. + * intent and markReady applies it right before the reveal. In `active` mode + * that means the window's first on-screen frame is already maximized. An + * `inactive` window cannot have both: the reveal that maximize() performs is + * an activating one, so it is revealed inactively first and then maximized, + * and the zoom is visible. Not taking the foreground is worth more than the + * single frame, and an E2E run has no saved maximized bounds to restore. * * `hidden` windows (e2e-fixture capture / E2E) never show, maximize, or take * focus from any path — captures run while the developer works elsewhere. @@ -157,8 +167,9 @@ export function createWindowRevealGate(mode: WindowRevealMode): WindowRevealGate }, markReady(win) { ready = true; - // Maximize first: it implicitly shows the window, so the reveal below - // becomes a no-op and the first visible frame is already maximized. + // Maximize first: in `active` mode it implicitly shows the window, so + // the reveal below becomes a no-op and the first visible frame is + // already maximized. if (pendingMaximize) { pendingMaximize = false; maximizeNow(win); diff --git a/scripts/desktop-real-window-smoke.mjs b/scripts/desktop-real-window-smoke.mjs index a2566a50ae..9166392dd2 100644 --- a/scripts/desktop-real-window-smoke.mjs +++ b/scripts/desktop-real-window-smoke.mjs @@ -31,7 +31,7 @@ import { execFile, spawn } from 'node:child_process'; import { terminateChildProcessTree } from '@maka/runtime/process-tree-terminator'; import { closeElectronApplication } from './electron-lifecycle.mjs'; -import { buildFixtureEnv } from './fixture-env.mjs'; +import { buildFixtureEnv, inactiveWindowPlatformArgs } from './fixture-env.mjs'; import { existsSync } from 'node:fs'; import { mkdir, readFile, rm, writeFile } from 'node:fs/promises'; import { createInterface } from 'node:readline/promises'; @@ -315,7 +315,9 @@ async function launchElectron(args, diagnostics) { env.MAKA_E2E_FIXTURE_WIDTH = String(args.width); env.MAKA_E2E_FIXTURE_HEIGHT = String(args.height); env.MAKA_REAL_WINDOW_SMOKE = '1'; - const launchArgs = ['.', `--user-data-dir=${userDataDir}`]; + // This run always reveals its window inactively, which needs XWayland on a + // native Wayland session. + const launchArgs = ['.', ...inactiveWindowPlatformArgs(), `--user-data-dir=${userDataDir}`]; const child = spawn(electronBin, launchArgs, { cwd: DESKTOP_DIR, env, diff --git a/scripts/fixture-env.mjs b/scripts/fixture-env.mjs index 1e1876c0c2..90e54e54bc 100644 --- a/scripts/fixture-env.mjs +++ b/scripts/fixture-env.mjs @@ -131,3 +131,26 @@ export function buildFixtureEnv(userDataDir, homeDir, options = {}) { export function isCiLinuxDisplay(env = process.env, platform = process.platform) { return Boolean(env.CI) && platform === 'linux'; } + +/** + * Extra Electron arguments a launch needs when its window will be revealed + * inactively. + * + * Electron 43 defaults to native Wayland when XDG_SESSION_TYPE=wayland, where + * BrowserWindow.showInactive() is unsupported — the window may simply not + * appear, which puts back the ~1fps compositor throttling and the geometry + * failures that asking for a visible window exists to avoid. Keep those + * launches on XWayland; every other launch retains Electron's platform + * default. + * + * Returns only the extra arguments, so each launcher composes it with its own: + * `['.', ...inactiveWindowPlatformArgs(), `--user-data-dir=${dir}`]`. + * + * @param {NodeJS.ProcessEnv} [env] + * @param {NodeJS.Platform} [platform] + */ +export function inactiveWindowPlatformArgs(env = process.env, platform = process.platform) { + return platform === 'linux' && env.XDG_SESSION_TYPE?.toLowerCase() === 'wayland' + ? ['--ozone-platform=x11'] + : []; +} diff --git a/scripts/fixture-env.test.mjs b/scripts/fixture-env.test.mjs new file mode 100644 index 0000000000..ed4a7c1bb6 --- /dev/null +++ b/scripts/fixture-env.test.mjs @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { inactiveWindowPlatformArgs, isCiLinuxDisplay } from './fixture-env.mjs'; + +describe('inactiveWindowPlatformArgs', () => { + it('keeps a native Wayland session on XWayland', () => { + // showInactive() is unsupported on native Wayland, so a window revealed + // inactively there may never appear. + assert.deepEqual(inactiveWindowPlatformArgs({ XDG_SESSION_TYPE: 'wayland' }, 'linux'), [ + '--ozone-platform=x11', + ]); + assert.deepEqual(inactiveWindowPlatformArgs({ XDG_SESSION_TYPE: 'Wayland' }, 'linux'), [ + '--ozone-platform=x11', + ]); + }); + + it("leaves every other launch on Electron's platform default", () => { + assert.deepEqual(inactiveWindowPlatformArgs({ XDG_SESSION_TYPE: 'x11' }, 'linux'), []); + assert.deepEqual(inactiveWindowPlatformArgs({}, 'linux'), []); + // CI's Xvfb display is X11, and macOS and Windows have no ozone platform. + assert.deepEqual(inactiveWindowPlatformArgs({ XDG_SESSION_TYPE: 'wayland' }, 'darwin'), []); + assert.deepEqual(inactiveWindowPlatformArgs({ XDG_SESSION_TYPE: 'wayland' }, 'win32'), []); + }); + + it('returns only the extra arguments, so a launcher keeps its own', () => { + const args = ['.', ...inactiveWindowPlatformArgs({}, 'darwin'), '--user-data-dir=/tmp/x']; + assert.deepEqual(args, ['.', '--user-data-dir=/tmp/x']); + }); +}); + +describe('isCiLinuxDisplay', () => { + it('is the isolated CI Linux display and nothing else', () => { + assert.equal(isCiLinuxDisplay({ CI: '1' }, 'linux'), true); + assert.equal(isCiLinuxDisplay({}, 'linux'), false); + assert.equal(isCiLinuxDisplay({ CI: '1' }, 'darwin'), false); + }); +}); diff --git a/scripts/fixture-window.mjs b/scripts/fixture-window.mjs index 148289d29e..b264aba6d3 100644 --- a/scripts/fixture-window.mjs +++ b/scripts/fixture-window.mjs @@ -38,7 +38,7 @@ import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { _electron as electron } from '@playwright/test'; import { closeElectronApplication } from './electron-lifecycle.mjs'; -import { buildFixtureEnv, isCiLinuxDisplay } from './fixture-env.mjs'; +import { buildFixtureEnv, inactiveWindowPlatformArgs, isCiLinuxDisplay } from './fixture-env.mjs'; const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); const DESKTOP_DIR = join(ROOT, 'apps', 'desktop'); @@ -57,20 +57,6 @@ const CLOSE_GRACE_MS = 5_000; */ export const CAPTURE_TIMEZONE = process.env.FIXTURE_TIMEZONE ?? 'UTC'; -/** - * Electron 43 defaults to native Wayland when XDG_SESSION_TYPE=wayland, where - * BrowserWindow.showInactive() is unsupported. Keep inactive fixtures on - * XWayland; other launches retain Electron's platform default. - * - * @param {NodeJS.ProcessEnv} [env] - * @param {NodeJS.Platform} [platform] - */ -export function inactiveWindowElectronArgs(env = process.env, platform = process.platform) { - return platform === 'linux' && env.XDG_SESSION_TYPE?.toLowerCase() === 'wayland' - ? ['.', '--ozone-platform=x11'] - : ['.']; -} - /** * Map the BrowserWindow owned by this launch without focusing it. * @@ -160,7 +146,8 @@ export async function withFixtureWindow(scenario, options, fn) { // xvfb throttles a hidden window's compositor to ~1fps; only that isolated // display gets a visible window. Local hit tests stay accessory/Dock-hidden. const ciVisible = isCiLinuxDisplay(); - const launchArgs = mapWindowInactive && !ciVisible ? inactiveWindowElectronArgs() : ['.']; + const launchArgs = + mapWindowInactive && !ciVisible ? ['.', ...inactiveWindowPlatformArgs()] : ['.']; const userDataDir = await mkdtemp(join(tmpdir(), 'maka-fixture-')); // Inside the throwaway userData dir so the same teardown removes it; there