diff --git a/tests/e2e/desktop-published-split-orientation-legacy-leaf.spec.ts b/tests/e2e/desktop-published-split-orientation-legacy-leaf.spec.ts index 0a0ff09f07a..514cb7dab9c 100644 --- a/tests/e2e/desktop-published-split-orientation-legacy-leaf.spec.ts +++ b/tests/e2e/desktop-published-split-orientation-legacy-leaf.spec.ts @@ -34,12 +34,14 @@ import { /** Legacy pane id shape: not a stable pane UUID, so it never reaches the published leaf set. */ const LEGACY_LEAF_ID = 'pane:9' -/** - * Shrinks both the cold-park delay and the hot-retain window. Set at module scope because the - * `orcaPage` fixture launches the app before any test body runs. - */ +/** Shrinks both the cold-park delay and the hot-retain window. */ const PARK_DELAY_MS = 2_000 -process.env.ORCA_E2E_TERMINAL_PARKING_DELAY_MS ??= String(PARK_DELAY_MS) +const PARK_DELAY_ENV = { ORCA_E2E_TERMINAL_PARKING_DELAY_MS: String(PARK_DELAY_MS) } + +// Why a fixture option and not `process.env`: a Playwright worker runs many spec files in one +// process, so a module-scope env write outlives this file and shrinks parking for every spec +// that follows it in the same worker. +test.use({ orcaAppExtraEnv: PARK_DELAY_ENV }) function collectLeafIds(node: TerminalPaneLayoutNode | null | undefined): string[] { if (!node) { @@ -174,7 +176,11 @@ test('publishes an unmounted split with its real orientation when a legacy leaf ) const offer = await createRuntimeDesktopPairingOffer(orcaPage) - client = await launchPairedElectronClient(offer, testInfo, 'legacy-leaf-orientation-observer') + // The observer inherited the same override back when it came from `process.env`; keep it so + // scoping the write to this file does not also change what the client does. + client = await launchPairedElectronClient(offer, testInfo, 'legacy-leaf-orientation-observer', { + extraEnv: PARK_DELAY_ENV + }) await waitForPairedClientWorktree(client.page, worktreeId) await expect diff --git a/tests/e2e/e2e-worker-env-isolation.unit.test.ts b/tests/e2e/e2e-worker-env-isolation.unit.test.ts new file mode 100644 index 00000000000..23b48f55afe --- /dev/null +++ b/tests/e2e/e2e-worker-env-isolation.unit.test.ts @@ -0,0 +1,91 @@ +import { readFileSync, readdirSync, statSync } from 'node:fs' +import { join, relative, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * One Playwright worker imports many spec files into one Node process, and the app fixtures + * launch Electron with a spread of that process's `process.env`. A module-scope write therefore + * reconfigures every app launched by every spec that follows in the same worker — and it runs at + * import time, before any hook or `finally` exists that could undo it, so unlike a write inside a + * test body it cannot be restored at all. + * + * That is not a hypothetical: a parking-delay override written this way shrank the terminal + * cold-park delay from 30s to 2s for later specs, which unmounted panes those specs still needed. + * Use `test.use({ orcaAppExtraEnv })`, which Playwright scopes to the file. + */ +const E2E_ROOT = resolve(__dirname) + +/** + * Module scope is read off column 0. The tree is prettier-formatted, so every statement nested in + * a function, hook, block, or `app.evaluate` callback is indented — including the in-body writes + * that legitimately save and restore around a relaunch. A write that starts a line is top-level. + */ +const MODULE_SCOPE_ENV_WRITE = + /^(?:process\.env\.[A-Za-z_][A-Za-z0-9_]*\s*(?:\??\|\||\?\?|)=[^=]|process\.env\[|delete\s+process\.env[.[]|Object\.assign\(\s*process\.env)/ + +/** + * The count of files writing `process.env` at module scope. + * + * May only ever be DECREASED. Raising it is never the fix: the replacement is a fixture option, + * which is strictly more capable here because it reaches the app launch without touching the + * worker every other spec shares. + */ +const MODULE_SCOPE_ENV_WRITER_PIN = 0 + +const SCANNED_EXTENSIONS = ['.ts', '.tsx'] +const IGNORED_DIRECTORIES = new Set(['node_modules', 'dist', 'out', 'build', '__fixtures__']) + +function collectE2eFiles(root: string): string[] { + const found: string[] = [] + for (const entry of readdirSync(root)) { + if (IGNORED_DIRECTORIES.has(entry)) { + continue + } + const path = join(root, entry) + if (statSync(path).isDirectory()) { + found.push(...collectE2eFiles(path)) + } else if (SCANNED_EXTENSIONS.some((extension) => path.endsWith(extension))) { + found.push(path) + } + } + return found +} + +function findModuleScopeEnvWrites(path: string): string[] { + return readFileSync(path, 'utf8') + .split('\n') + .flatMap((line, index) => + MODULE_SCOPE_ENV_WRITE.test(line) + ? [`${relative(E2E_ROOT, path)}:${index + 1}: ${line.trim()}`] + : [] + ) +} + +describe('e2e worker env isolation', () => { + const offenders = collectE2eFiles(E2E_ROOT).flatMap(findModuleScopeEnvWrites) + + it('no e2e file writes process.env at module scope', () => { + expect(offenders).toEqual([]) + }) + + it('holds the module-scope env writer count at its ratchet', () => { + const files = new Set(offenders.map((offender) => offender.split(':')[0])) + expect(files.size).toBeLessThanOrEqual(MODULE_SCOPE_ENV_WRITER_PIN) + }) + + it('detects the shape it is meant to catch', () => { + // Guards the regex itself: a green that cannot go red would pass this whole file forever. + expect(MODULE_SCOPE_ENV_WRITE.test("process.env.ORCA_E2E_X ??= '1'")).toBe(true) + expect(MODULE_SCOPE_ENV_WRITE.test("process.env.ORCA_E2E_X = '1'")).toBe(true) + expect(MODULE_SCOPE_ENV_WRITE.test('delete process.env.ORCA_E2E_X')).toBe(true) + expect(MODULE_SCOPE_ENV_WRITE.test("Object.assign(process.env, { ORCA_E2E_X: '1' })")).toBe( + true + ) + // Reads, and writes nested in any body, stay legal. + expect(MODULE_SCOPE_ENV_WRITE.test('const x = Number(process.env.ORCA_E2E_X) || 500')).toBe( + false + ) + expect(MODULE_SCOPE_ENV_WRITE.test(" process.env.ORCA_E2E_X = '1'")).toBe(false) + expect(MODULE_SCOPE_ENV_WRITE.test("if (process.env.ORCA_E2E_X === '1') {")).toBe(false) + }) +}) diff --git a/tests/e2e/helpers/terminal-accessibility-tree.ts b/tests/e2e/helpers/terminal-accessibility-tree.ts new file mode 100644 index 00000000000..b2520f2bc77 --- /dev/null +++ b/tests/e2e/helpers/terminal-accessibility-tree.ts @@ -0,0 +1,46 @@ +import type { Page } from '@stablyai/playwright-test' +import { expect } from '@stablyai/playwright-test' + +/** + * Reads a tab's `.xterm-accessibility-tree` text, enabling screen-reader mode on the pane it + * finds each time it is called. + * + * Why re-apply rather than set it once up front: `screenReaderMode` is an option on the xterm + * instance, and the node it renders belongs to that instance's DOM. A pane that parks and + * remounts, or rebinds after a reconnect, comes back as a new instance with the option off and + * no tree — so a one-shot mutation before the read stops producing the very node the read is + * waiting for, and the wait fails as "element not found" rather than as a content mismatch. + * + * Returns null when the pane or the node is not there yet, so a poll keeps retrying. + */ +export function readTerminalAccessibilityText(page: Page, tabId: string): Promise { + return page.evaluate((id) => { + const manager = window.__paneManagers?.get(id) + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] + if (!pane) { + return null + } + if (!pane.terminal.options.screenReaderMode) { + pane.terminal.options.screenReaderMode = true + pane.terminal.refresh(0, pane.terminal.rows - 1) + } + const node = document.querySelector( + `[data-terminal-tab-id="${CSS.escape(id)}"] .xterm-accessibility-tree` + ) + return node?.textContent ?? null + }, tabId) +} + +export async function expectTerminalAccessibilityText( + page: Page, + tabId: string, + expected: string, + timeoutMs = 30_000 +): Promise { + await expect + .poll(() => readTerminalAccessibilityText(page, tabId), { + timeout: timeoutMs, + message: `terminal tab ${tabId} never rendered ${expected} in its accessibility tree` + }) + .toContain(expected) +} diff --git a/tests/e2e/helpers/terminal.ts b/tests/e2e/helpers/terminal.ts index 221a65a2e59..bf198ad9f6d 100644 --- a/tests/e2e/helpers/terminal.ts +++ b/tests/e2e/helpers/terminal.ts @@ -25,6 +25,11 @@ import { waitForTerminalOutput as waitForTerminalOutputImpl } from './terminal-pane-operations' +export { + expectTerminalAccessibilityText, + readTerminalAccessibilityText +} from './terminal-accessibility-tree' + export { getTerminalContent, readPaneIdentitySnapshot, diff --git a/tests/e2e/ssh-cold-activation-restore.spec.ts b/tests/e2e/ssh-cold-activation-restore.spec.ts index ebe1bd88edc..82d586bdb85 100644 --- a/tests/e2e/ssh-cold-activation-restore.spec.ts +++ b/tests/e2e/ssh-cold-activation-restore.spec.ts @@ -2,6 +2,7 @@ import type { ElectronApplication } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' import { + expectTerminalAccessibilityText, focusActiveTerminalInput, waitForActivePanePtyId, waitForActiveTerminalManager @@ -193,26 +194,12 @@ test.describe('SSH cold activation restore', () => { } ) .toBe(firstTabId) - await orcaPage.evaluate((tabId) => { - const manager = window.__paneManagers?.get(tabId) - const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] - if (!pane) { - throw new Error('Restored SSH pane unavailable') - } - pane.terminal.options.screenReaderMode = true - pane.terminal.refresh(0, pane.terminal.rows - 1) - }, firstTabId) - const marker = `SSH_RESTORE_OK_${Date.now()}` const proofFile = '/tmp/orca-ssh-restore-proof' await focusActiveTerminalInput(orcaPage) await orcaPage.keyboard.type(`printf '${marker}' > ${proofFile} && printf '${marker}\\n'`) await orcaPage.keyboard.press('Enter') - await expect( - orcaPage.locator( - `[data-terminal-tab-id=${JSON.stringify(firstTabId)}] .xterm-accessibility-tree` - ) - ).toContainText(marker, { timeout: 30_000 }) + await expectTerminalAccessibilityText(orcaPage, firstTabId, marker) expect(execDockerSshRelayTargetCommand(target, `cat ${proofFile}`)).toBe(marker) } finally { cleanupDockerSshRelayTarget(target) @@ -292,27 +279,13 @@ test.describe('SSH cold activation restore', () => { .toBe(remote.worktreeId) await waitForActiveTerminalManager(secondLaunch.page, 60_000) expect(await waitForActivePanePtyId(secondLaunch.page, 60_000)).toBe(firstPtyId) - await secondLaunch.page.evaluate((tabId) => { - const manager = window.__paneManagers?.get(tabId) - const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] - if (!pane) { - throw new Error('Restored SSH pane unavailable') - } - pane.terminal.options.screenReaderMode = true - pane.terminal.refresh(0, pane.terminal.rows - 1) - }, restoredTabId) - const restoredMarker = `SSH_OWNER_RESTORED_${Date.now()}` await focusActiveTerminalInput(secondLaunch.page) await secondLaunch.page.keyboard.type( `printf '%s|%s|%s|%s\\n' "$$" "$ORCA_BG_PID" "$ORCA_RESTART_TOKEN" "$PWD" > ${afterProofPath}; printf '${restoredMarker}\\n'` ) await secondLaunch.page.keyboard.press('Enter') - await expect( - secondLaunch.page.locator( - `[data-terminal-tab-id=${JSON.stringify(restoredTabId)}] .xterm-accessibility-tree` - ) - ).toContainText(restoredMarker, { timeout: 30_000 }) + await expectTerminalAccessibilityText(secondLaunch.page, restoredTabId, restoredMarker) await expect.poll(() => readRemoteProof(target!, afterProofPath)).toBe(beforeProof) } finally { if (secondApp) {