From e7dc9b60995d73a0206c34891188e45cd9b708f2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 18:41:24 -0700 Subject: [PATCH] test: honor background launch in paired client window helpers (#18978) --- AGENTS.md | 6 +++ tests/AGENTS.md | 5 +- .../helpers/paired-client-window-reveal.ts | 15 ++++-- .../paired-client-window-reveal.unit.test.ts | 47 ++++++++++++++++++- 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8b0156ba6b1..306c9c8d5ed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,12 @@ All UI work — layout, color, typography, spacing, component selection, UX beha ## Electron UI Validation +Always run tests and agent-launched apps in the background with `ORCA_BACKGROUND_LAUNCH=1`. +Never steal monitor focus or reveal test windows: no `show()`, `showInactive()`, `bringToFront()`, +`app.focus()`, or OS activation. Use CDP screenshots of hidden renderers. Keep native-focus and +visible-window tests paused on the user's desktop; run them on an isolated display or CI. +Rebuild modified launch-policy code before running an app; stale build wrappers are not safe. + Use the `$electron` skill and Playwright CDP for rendered Orca UI checks. Do not use computer-use for Orca UI validation. # Style diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 26987a87c25..30f522652fe 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -18,6 +18,5 @@ Rules when adding tests or scripts: - Do not reveal windows in explicit background or headless runs. Only an explicitly headful run may call `showInactive()`; never call `show()` or `bringToFront()` in automated background checks. - Tag a spec `@headful` only when it needs real pixels; it still runs in the background. -- `ORCA_E2E_FOREGROUND=1` is the only opt-out, for runs whose subject _is_ native focus (IME and - other OS-level key injection). Clear `ORCA_BACKGROUND_LAUNCH` for that isolated run and add a - comment saying why; an explicit background request takes precedence. +- Native-focus tests belong on an isolated display or CI. Do not set `ORCA_E2E_FOREGROUND=1` + on the user’s desktop; it cannot override explicit background mode. diff --git a/tests/e2e/helpers/paired-client-window-reveal.ts b/tests/e2e/helpers/paired-client-window-reveal.ts index 302d573c3d1..1ec5b635d77 100644 --- a/tests/e2e/helpers/paired-client-window-reveal.ts +++ b/tests/e2e/helpers/paired-client-window-reveal.ts @@ -29,22 +29,24 @@ export function assertPairedClientWindowRevealed(report: PairedClientWindowRevea export type PairedClientWindowFocusReport = PairedClientWindowRevealReport & { isFocused: boolean } /** - * Brings a paired client to the front, which a launched-but-background window never is. Main-side - * policies that ask whether the reader is looking at a WebContents read the OS focus state, so a - * spec driving real presses through such a policy has to put the window there first. + * Native-focus coverage must run on an isolated display or CI, never in background mode. */ export async function focusPairedClientWindow( client: RevealablePairedClient, { timeoutMs = 15_000 }: { timeoutMs?: number } = {} ): Promise { + await client.app.evaluate(() => { + if (process.env.ORCA_BACKGROUND_LAUNCH === '1') { + throw new Error('Native focus is forbidden by ORCA_BACKGROUND_LAUNCH') + } + }) const revealed = await revealPairedClientWindow(client) const deadline = Date.now() + timeoutMs let isFocused = false while (!isFocused) { isFocused = await client.app.evaluate(({ app, BrowserWindow }) => { const window = BrowserWindow.getAllWindows()[0] - // Why steal: nothing else in the run is asking for the front, and the window manager keeps - // the launching terminal there otherwise. + // Native-focus coverage requires a dedicated foreground session. app.focus({ steal: true }) window?.focus() return window?.isFocused() ?? false @@ -61,6 +63,9 @@ export async function revealPairedClientWindow( client: RevealablePairedClient ): Promise { const report = await client.app.evaluate(({ BrowserWindow }) => { + if (process.env.ORCA_BACKGROUND_LAUNCH === '1') { + throw new Error('Window reveal is forbidden by ORCA_BACKGROUND_LAUNCH') + } const windows = BrowserWindow.getAllWindows() const window = windows[0] const wasVisible = window?.isVisible() ?? false diff --git a/tests/e2e/helpers/paired-client-window-reveal.unit.test.ts b/tests/e2e/helpers/paired-client-window-reveal.unit.test.ts index dfb83e4c441..706088e6762 100644 --- a/tests/e2e/helpers/paired-client-window-reveal.unit.test.ts +++ b/tests/e2e/helpers/paired-client-window-reveal.unit.test.ts @@ -1,5 +1,10 @@ -import { describe, expect, it } from 'vitest' -import { assertPairedClientWindowRevealed } from './paired-client-window-reveal' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + assertPairedClientWindowRevealed, + focusPairedClientWindow, + revealPairedClientWindow, + type RevealablePairedClient +} from './paired-client-window-reveal' describe('assertPairedClientWindowRevealed', () => { it('accepts a window that the reveal made visible', () => { @@ -42,3 +47,41 @@ describe('assertPairedClientWindowRevealed', () => { ).toThrow(/stayed hidden after showInactive\(\)/) }) }) + +describe('paired client background safety', () => { + afterEach(() => vi.unstubAllEnvs()) + + function makeClient() { + const showInactive = vi.fn() + const focus = vi.fn() + const getAllWindows = vi.fn(() => [{ isVisible: () => false, showInactive, focus }]) + const evaluate = vi.fn(async (callback) => + callback({ + app: { focus }, + BrowserWindow: { getAllWindows } + }) + ) + const client = { + app: { evaluate }, + page: { waitForFunction: vi.fn() } + } as unknown as RevealablePairedClient + return { client, showInactive, focus, getAllWindows } + } + + it('rejects an explicit reveal before touching native windows', async () => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', '1') + const { client, getAllWindows, showInactive } = makeClient() + await expect(revealPairedClientWindow(client)).rejects.toThrow('Window reveal is forbidden') + expect(getAllWindows).not.toHaveBeenCalled() + expect(showInactive).not.toHaveBeenCalled() + }) + + it.each(['0', '1'])('rejects focus in background mode with foreground=%s', async (foreground) => { + vi.stubEnv('ORCA_BACKGROUND_LAUNCH', '1') + vi.stubEnv('ORCA_E2E_FOREGROUND', foreground) + const { client, focus, getAllWindows } = makeClient() + await expect(focusPairedClientWindow(client)).rejects.toThrow('Native focus is forbidden') + expect(getAllWindows).not.toHaveBeenCalled() + expect(focus).not.toHaveBeenCalled() + }) +})