From f116d2ca2abebb5c8c73a2d2eb253a91beb8cd04 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:53:01 -0700 Subject: [PATCH] test(ci): retry Windows teardown EPERM and restart evaluate misses (#17780) Restart-survival polls treated a recycled renderer as a hard failure. Wrap those evaluates so "Execution context was destroyed" is a pending miss. Windows package-lane teardowns after a force-kill used rmSync with force:true only, which does not absorb EPERM; put them on the shared maxRetries:8 policy. --- .../rebuild-native-deps-node-pty.test.mjs | 19 +- config/scripts/rebuild-native-deps.test.mjs | 19 +- .../windows-hook-payload-delivery.test.ts | 5 +- .../cli/wsl-cli-powershell-boundary.test.ts | 5 +- .../codex-managed-home-lifecycle.ts | 8 +- src/main/cursor/hook-service.test.ts | 7 +- src/main/host-tree-removal.ts | 22 +- .../orca-profiles/profile-index-store.test.ts | 5 +- .../repo-worktree-admin-fingerprint.test.ts | 7 +- .../windows/windows-host-job.win32.test.ts | 5 +- .../windows-command-line.win32.test.ts | 3 +- src/shared/secure-file-fsync-flags.test.ts | 5 +- ...windows-lane-tree-removal-boundary.test.ts | 171 ++++++++++ .../windows-transient-lock-removal.test.ts | 117 +++++++ src/shared/windows-transient-lock-removal.ts | 64 ++++ .../helpers/client-hosted-browser-fixture.ts | 157 +++++---- ...client-hosted-browser-fixture.unit.test.ts | 41 +++ ...nt-hosted-browser-restart-survival.spec.ts | 321 +----------------- 18 files changed, 548 insertions(+), 433 deletions(-) create mode 100644 src/shared/windows-lane-tree-removal-boundary.test.ts create mode 100644 src/shared/windows-transient-lock-removal.test.ts create mode 100644 src/shared/windows-transient-lock-removal.ts create mode 100644 tests/e2e/helpers/client-hosted-browser-fixture.unit.test.ts diff --git a/config/scripts/rebuild-native-deps-node-pty.test.mjs b/config/scripts/rebuild-native-deps-node-pty.test.mjs index 665ac0312ab..09e38853371 100644 --- a/config/scripts/rebuild-native-deps-node-pty.test.mjs +++ b/config/scripts/rebuild-native-deps-node-pty.test.mjs @@ -1,4 +1,5 @@ -import { existsSync, readFileSync, rmSync } from 'node:fs' +import { existsSync, readFileSync } from 'node:fs' +import { removeTreeSync } from '../../src/shared/windows-transient-lock-removal.ts' import { join } from 'node:path' import { describe, expect, it } from 'vitest' @@ -44,7 +45,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => { ) expect(existsSync(rebuildLogPath)).toBe(false) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } } ) @@ -80,7 +81,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => { ) ).toBe('// napi.h\n') } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -104,7 +105,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => { expect(readFileSync(join(runtimeDir, 'conpty.dll'), 'utf8')).toBe('conpty.dll x64') expect(readFileSync(join(runtimeDir, 'OpenConsole.exe'), 'utf8')).toBe('OpenConsole.exe x64') } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -132,7 +133,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => { const rebuildCall = JSON.parse(readFileSync(rebuildLogPath, 'utf8').trim()) expect(rebuildCall.onlyModules).toEqual(['windows-native-registry']) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } } ) @@ -162,7 +163,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => { const rebuildCall = JSON.parse(readFileSync(rebuildLogPath, 'utf8').trim()) expect(rebuildCall.onlyModules).toEqual(['node-pty']) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } } ) @@ -193,7 +194,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => { expect(rebuildCall.ignoreModules).toEqual(['cpu-features']) expect(rebuildCall.force).toBe(true) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } } ) @@ -221,7 +222,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => { ) expect(existsSync(rebuildLogPath)).toBe(false) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } } ) @@ -251,7 +252,7 @@ describe('rebuild-native-deps patched node-pty rebuild', () => { expect(rebuildCall.onlyModules).toEqual(['node-pty']) expect(rebuildCall.force).toBe(true) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } } ) diff --git a/config/scripts/rebuild-native-deps.test.mjs b/config/scripts/rebuild-native-deps.test.mjs index cddab2ee9f0..d4db08d3e2e 100644 --- a/config/scripts/rebuild-native-deps.test.mjs +++ b/config/scripts/rebuild-native-deps.test.mjs @@ -1,6 +1,7 @@ import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { describe, expect, it } from 'vitest' +import { removeTreeSync } from '../../src/shared/windows-transient-lock-removal.ts' import { mkTempProject, @@ -36,7 +37,7 @@ describe('rebuild-native-deps Electron install fallback', () => { 'download attempted\n' ) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -60,7 +61,7 @@ describe('rebuild-native-deps Electron install fallback', () => { 'Continuing postinstall because Electron binary installation failed' ) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -81,7 +82,7 @@ describe('rebuild-native-deps Electron install fallback', () => { 'Continuing postinstall because Electron binary installation failed' ) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -117,7 +118,7 @@ describe('rebuild-native-deps Electron install fallback', () => { 'stale-path' ) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -141,7 +142,7 @@ describe('rebuild-native-deps Electron install fallback', () => { 'platform=linux arch=arm64\ndownload attempted\n' ) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -162,7 +163,7 @@ describe('rebuild-native-deps Electron install fallback', () => { expect(result.status, result.stderr).toBe(0) expect(existsSync(join(projectDir, 'electron-get.log'))).toBe(false) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -188,7 +189,7 @@ describe('rebuild-native-deps Electron install fallback', () => { 'electron.exe' ) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -209,7 +210,7 @@ describe('rebuild-native-deps Electron install fallback', () => { 'platform=linux arch=x64' ) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) @@ -230,7 +231,7 @@ describe('rebuild-native-deps Electron install fallback', () => { expect(result.stdout).toContain('Repaired Electron path.txt -> electron') expect(existsSync(join(projectDir, 'electron-get.log'))).toBe(false) } finally { - rmSync(projectDir, { recursive: true, force: true }) + removeTreeSync(projectDir) } }) }) diff --git a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts index aabf2545172..22103b178ff 100644 --- a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts +++ b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts @@ -7,7 +7,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { spawn } from 'node:child_process' import { createServer, type Server } from 'node:http' -import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { mkdtempSync, readFileSync } from 'node:fs' +import { removeTreeSync } from '../../shared/windows-transient-lock-removal' import { tmpdir } from 'node:os' import { join } from 'node:path' import type * as osModule from 'node:os' @@ -149,7 +150,7 @@ describe.skipIf(process.platform !== 'win32')('Windows managed hook payload deli server = null homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) if (home) { - rmSync(home, { recursive: true, force: true }) + removeTreeSync(home) home = '' } }) diff --git a/src/main/cli/wsl-cli-powershell-boundary.test.ts b/src/main/cli/wsl-cli-powershell-boundary.test.ts index 7e706a0ed12..ddfdabe9ad3 100644 --- a/src/main/cli/wsl-cli-powershell-boundary.test.ts +++ b/src/main/cli/wsl-cli-powershell-boundary.test.ts @@ -1,5 +1,6 @@ import { spawnSync } from 'node:child_process' -import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, writeFile } from 'node:fs/promises' +import { removeTree } from '../../shared/windows-transient-lock-removal' import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, expect, it } from 'vitest' @@ -113,7 +114,7 @@ describe('WSL CLI PowerShell boundary', () => { expect(exitResult.error).toBeUndefined() expect(exitResult.status).toBe(23) } finally { - await rm(root, { recursive: true, force: true }) + await removeTree(root) } } ) diff --git a/src/main/codex-accounts/codex-managed-home-lifecycle.ts b/src/main/codex-accounts/codex-managed-home-lifecycle.ts index b86a65762a1..c4790869a99 100644 --- a/src/main/codex-accounts/codex-managed-home-lifecycle.ts +++ b/src/main/codex-accounts/codex-managed-home-lifecycle.ts @@ -1,6 +1,10 @@ import { mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs' import { dirname, join, resolve, sep } from 'node:path' import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence' +import { + WINDOWS_RM_MAX_RETRIES, + WINDOWS_RM_RETRY_DELAY_MS +} from '../../shared/windows-transient-lock-removal' import { quotePosixShell } from '../../shared/wsl-login-shell-command' import { parseWslUncPath } from '../../shared/wsl-paths' import { toWindowsWslPath } from '../wsl' @@ -10,10 +14,6 @@ import { writeFileAtomically } from './fs-utils' import { ManagedCodexHomeTemporarilyUnavailableError } from './host-codex-managed-home-ownership' import type { CodexManagedHomePath } from './codex-managed-home-path' -// Why: mirrors the Windows rm retry policy in local-worktree-filesystem — a -// just-terminated codex login can briefly keep handles inside a managed home. -const WINDOWS_RM_MAX_RETRIES = 8 -const WINDOWS_RM_RETRY_DELAY_MS = 150 const WSL_MANAGED_HOME_TIMEOUT_MS = 5_000 function removeManagedHomeTreeSync(targetPath: string): void { diff --git a/src/main/cursor/hook-service.test.ts b/src/main/cursor/hook-service.test.ts index be9ce14af7e..f5f25295a60 100644 --- a/src/main/cursor/hook-service.test.ts +++ b/src/main/cursor/hook-service.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdirSync, mkdtempSync, readFileSync, rmSync, unlinkSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs' +import { removeTreeSync } from '../../shared/windows-transient-lock-removal' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { spawnSync } from 'node:child_process' @@ -98,7 +99,7 @@ describe('CursorHookService', () => { afterEach(() => { vi.clearAllMocks() - rmSync(homeDir, { recursive: true, force: true }) + removeTreeSync(homeDir) }) it('installs Cursor Agent hooks with the documented top-level command schema', () => { @@ -164,7 +165,7 @@ describe('CursorHookService', () => { expect(command).toMatch(WINDOWS_POWERSHELL_LAUNCHER) } } finally { - rmSync(spaceHome, { recursive: true, force: true }) + removeTreeSync(spaceHome) } } ) diff --git a/src/main/host-tree-removal.ts b/src/main/host-tree-removal.ts index 666d129b27c..a5d5d447956 100644 --- a/src/main/host-tree-removal.ts +++ b/src/main/host-tree-removal.ts @@ -2,16 +2,14 @@ // generations) hits the same Windows stickiness — AV/indexers/late handle releases surface transient // EBUSY/ENOTEMPTY/EPERM on a tree Node just emptied. One helper so no call site forgets the retries. -import type { RmOptions } from 'node:fs' import { rm } from 'node:fs/promises' import { win32 } from 'node:path' import { setTimeout as delay } from 'node:timers/promises' import { isWindowsAbsolutePathLike } from '../shared/cross-platform-path' import { isWslUncPath } from '../shared/wsl-paths' +import { transientLockRemovalOptions } from '../shared/windows-transient-lock-removal' const WINDOWS_REMOVE_RETRY_DELAYS_MS = [250, 500, 1_000, 2_000] -const WINDOWS_RM_MAX_RETRIES = 8 -const WINDOWS_RM_RETRY_DELAY_MS = 150 /** Convert a native host filesystem path to the Win32 long-path namespace. */ export function toHostFilesystemPath(targetPath: string): string { @@ -30,20 +28,6 @@ export function toHostRemovalPath(targetPath: string): string { return toHostFilesystemPath(targetPath) } -function getHostRemovalOptions(): RmOptions { - const base = { recursive: true, force: true } - if (process.platform !== 'win32') { - return base - } - return { - ...base, - // Why: large Windows trees commonly surface transient ENOTEMPTY/EPERM while - // Node walks and removes nested directories. - maxRetries: WINDOWS_RM_MAX_RETRIES, - retryDelay: WINDOWS_RM_RETRY_DELAY_MS - } -} - function isTransientWindowsRemovalError(error: unknown): boolean { if (process.platform !== 'win32' || typeof error !== 'object' || error === null) { return false @@ -60,7 +44,9 @@ function isTransientWindowsRemovalError(error: unknown): boolean { export async function removeHostTree(targetPath: string): Promise { const removalPath = toHostRemovalPath(targetPath) const retryDelays = process.platform === 'win32' ? WINDOWS_REMOVE_RETRY_DELAYS_MS : [] - const rmOptions = getHostRemovalOptions() + // Why: large Windows trees commonly surface transient ENOTEMPTY/EPERM while Node walks and + // removes nested directories; Node's own retries absorb that before the loop below has to. + const rmOptions = transientLockRemovalOptions() let attempt = 0 while (true) { diff --git a/src/main/orca-profiles/profile-index-store.test.ts b/src/main/orca-profiles/profile-index-store.test.ts index 06f2efe0903..1d02c4d7dcc 100644 --- a/src/main/orca-profiles/profile-index-store.test.ts +++ b/src/main/orca-profiles/profile-index-store.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { installFakeAppEnvironment } from '../../../config/scripts/vitest-host-ports-setup' -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync, mkdirSync } from 'node:fs' +import { existsSync, mkdtempSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs' +import { removeTreeSync } from '../../shared/windows-transient-lock-removal' import { join } from 'node:path' import { tmpdir } from 'node:os' import { @@ -35,7 +36,7 @@ describe('profile index store', () => { }) afterEach(() => { - rmSync(testState.dir, { recursive: true, force: true }) + removeTreeSync(testState.dir) }) it('creates the default local profile and copies legacy state without deleting it', async () => { diff --git a/src/main/runtime/repo-worktree-admin-fingerprint.test.ts b/src/main/runtime/repo-worktree-admin-fingerprint.test.ts index a8f2fc9d8e4..84b91f9c12f 100644 --- a/src/main/runtime/repo-worktree-admin-fingerprint.test.ts +++ b/src/main/runtime/repo-worktree-admin-fingerprint.test.ts @@ -4,7 +4,8 @@ // Windows path resolution, CRLF in `HEAD`/`gitdir`/`commondir`, and whether `worktree move`/`lock` // and deleting a live checkout behave as they do on POSIX. The Linux shards cannot reach any of it. import { execFile } from 'node:child_process' -import { mkdir, mkdtemp, realpath, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, realpath, writeFile } from 'node:fs/promises' +import { removeTree } from '../../shared/windows-transient-lock-removal' import { tmpdir } from 'node:os' import { join } from 'node:path' import { promisify } from 'node:util' @@ -43,7 +44,7 @@ beforeEach(async () => { }) afterEach(async () => { - await rm(scratchDir, { recursive: true, force: true }) + await removeTree(scratchDir) }) describe('readRepoWorktreeAdminFingerprint', () => { @@ -71,7 +72,7 @@ describe('readRepoWorktreeAdminFingerprint', () => { it('changes when a worktree directory is deleted outside Git', async () => { // The admin dir is untouched by `rm -rf`, but the row's `prunable` flag flips. const before = await fingerprint() - await rm(worktreePath, { recursive: true, force: true }) + await removeTree(worktreePath) expect(await fingerprint()).not.toBe(before) }) diff --git a/src/main/windows/windows-host-job.win32.test.ts b/src/main/windows/windows-host-job.win32.test.ts index 94e9c323860..941d81fd4d3 100644 --- a/src/main/windows/windows-host-job.win32.test.ts +++ b/src/main/windows/windows-host-job.win32.test.ts @@ -1,8 +1,9 @@ -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdtempSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { spawn } from 'node:child_process' import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { removeTreeSync } from '../../shared/windows-transient-lock-removal' /** * The second half of the two-job design. @@ -50,7 +51,7 @@ describeOnWindows('host job reaps the tree when the host dies', () => { }) afterAll(() => { - rmSync(dir, { recursive: true, force: true }) + removeTreeSync(dir) }) it('kills a pty and its detached grandchild when the host is force-killed', async () => { diff --git a/src/shared/child-process/windows-command-line.win32.test.ts b/src/shared/child-process/windows-command-line.win32.test.ts index 64ed8322ec9..8f5aa14660e 100644 --- a/src/shared/child-process/windows-command-line.win32.test.ts +++ b/src/shared/child-process/windows-command-line.win32.test.ts @@ -1,4 +1,5 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { removeTreeSync } from '../windows-transient-lock-removal' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterAll, beforeAll, describe, expect, it } from 'vitest' @@ -37,7 +38,7 @@ describeOnWindows('Windows .cmd argument round-trip', () => { }) afterAll(() => { - rmSync(dir, { recursive: true, force: true }) + removeTreeSync(dir) }) function decode(stdout: string): string[] { diff --git a/src/shared/secure-file-fsync-flags.test.ts b/src/shared/secure-file-fsync-flags.test.ts index 15505151aef..6cb7b0fe1fc 100644 --- a/src/shared/secure-file-fsync-flags.test.ts +++ b/src/shared/secure-file-fsync-flags.test.ts @@ -1,4 +1,5 @@ -import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, mkdtempSync, writeFileSync } from 'node:fs' +import { removeTreeSync } from './windows-transient-lock-removal' import type * as NodeFs from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -34,7 +35,7 @@ const createdPaths: string[] = [] afterEach(() => { openedPaths.length = 0 for (const path of createdPaths.splice(0)) { - rmSync(path, { recursive: true, force: true }) + removeTreeSync(path) } }) diff --git a/src/shared/windows-lane-tree-removal-boundary.test.ts b/src/shared/windows-lane-tree-removal-boundary.test.ts new file mode 100644 index 00000000000..dfacc4a9ba4 --- /dev/null +++ b/src/shared/windows-lane-tree-removal-boundary.test.ts @@ -0,0 +1,171 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * The Windows CI lane runs a fixed list of specs on `windows-2022`, and every one of them removes + * a temporary tree when it is done. On Windows those removals race a handle the OS has not + * released yet — a just-exited child, an indexer, a dlopen'd native module — so a raw + * `rmSync(dir, { recursive: true, force: true })` throws EPERM after the test's assertions have + * all passed, and the lane reports a green test as a failure. + * + * `removeTree`/`removeTreeSync` carry the repo's `maxRetries: 8` policy. This keeps the lane on + * them: a new spec that hand-rolls the removal fails here rather than intermittently on Windows. + */ +const REPO_ROOT = join(__dirname, '..', '..') +const WORKFLOW_PATH = join(REPO_ROOT, '.github', 'workflows', 'pr.yml') +const WINDOWS_STEP_NAME = 'Test Windows-specific boundaries' + +/** The spec paths the `package (windows)` job passes to vitest, read from the workflow itself. */ +function readWindowsLaneSpecs(): string[] { + const workflow = readFileSync(WORKFLOW_PATH, 'utf8') + const stepIndex = workflow.indexOf(`- name: ${WINDOWS_STEP_NAME}`) + expect(stepIndex, `${WORKFLOW_PATH} no longer has a "${WINDOWS_STEP_NAME}" step`).toBeGreaterThan( + -1 + ) + const nextStepIndex = workflow.indexOf('\n - name:', stepIndex + 1) + const step = workflow.slice(stepIndex, nextStepIndex === -1 ? undefined : nextStepIndex) + return step + .split('\n') + .map((line) => line.trim()) + .filter((line) => /^(src|tests|config)\/.+\.(test|spec)\.(ts|tsx|mjs)$/.test(line)) +} + +/** `node:fs` and `node:fs/promises`, spelled with or without the `node:` prefix. */ +const FS_SPECIFIER = String.raw`['"](?:node:)?fs(?:/promises)?['"]` +/** The `{ … }` clause of an fs import or require, which is where a rename would be declared. */ +const FS_BINDING_CLAUSE = new RegExp( + String.raw`\{([^}]*)\}\s*(?:from\s*${FS_SPECIFIER}|=\s*(?:await\s+import|require)\(\s*${FS_SPECIFIER})`, + 'g' +) +/** `rm as removeDir` or `rmSync: dropTree` — the two ways a binding gets a local name. */ +const RENAMED_REMOVAL = /\brm(?:Sync)?\s*(?:as|:)\s*([A-Za-z0-9_$]+)/g + +/** + * The local names a recursive removal can be called by in `source`. + * + * Namespaced spellings are covered by the optional `.` prefix in the matcher rather + * than by listing names, so `fsp.rm` and `fsPromises.rm` are caught without anyone having to teach + * the rule that spelling first. Renames are the one form that prefix cannot see, so they are read + * out of the import clause. + */ +function collectRemovalNames(source: string): string[] { + const names = new Set(['rmSync', 'rm']) + for (const clause of source.matchAll(FS_BINDING_CLAUSE)) { + for (const rename of clause[1].matchAll(RENAMED_REMOVAL)) { + names.add(rename[1]) + } + } + return [...names] +} + +/** Every recursive removal that does not go through the retrying helper. */ +function findRawRecursiveRemovals(source: string): number[] { + const offenders: number[] = [] + const call = new RegExp( + String.raw`(? { + const specs = readWindowsLaneSpecs() + + it('reads a non-trivial spec list out of the workflow', () => { + // A parser that silently matched nothing would make every assertion below vacuous. + expect(specs.length).toBeGreaterThan(10) + expect(specs).toContain('config/scripts/rebuild-native-deps.test.mjs') + expect(specs).toContain('src/main/windows/windows-host-job.win32.test.ts') + }) + + it('actually detects a raw recursive removal', () => { + // Without this the scan below passes for any reason at all, including not scanning. + expect(findRawRecursiveRemovals('rmSync(dir, { recursive: true, force: true })')).toEqual([1]) + expect( + findRawRecursiveRemovals( + 'await rm(dir, {\n recursive: true,\n force: true,\n maxRetries: 8\n})' + ) + ).toEqual([]) + // A single-file removal is not this rule's business. + expect(findRawRecursiveRemovals('rmSync(file, { force: true })')).toEqual([]) + }) + + it('detects the removal whatever the import spelled it', () => { + // Why: a rule that only reads one import style stops catching violations the moment someone + // writes the next one differently — and the guard goes on reporting zero offenders. + const spellings: [string, string][] = [ + ['bare named import', "import { rmSync } from 'node:fs'\nrmSync(DIR"], + ['fs namespace', "import * as fs from 'node:fs'\nfs.rmSync(DIR"], + ['fsp namespace', "import * as fsp from 'node:fs/promises'\nawait fsp.rm(DIR"], + [ + 'fsPromises namespace', + "import * as fsPromises from 'node:fs/promises'\nawait fsPromises.rm(DIR" + ], + ['nodeFs namespace', "import * as nodeFs from 'node:fs'\nnodeFs.rmSync(DIR"], + ['unprefixed fs specifier', "import * as fs from 'fs'\nfs.rmSync(DIR"], + [ + 'renamed named import', + "import { rm as removeDir } from 'node:fs/promises'\nawait removeDir(DIR" + ], + ['renamed require', "const { rmSync: dropTree } = require('node:fs')\ndropTree(DIR"] + ] + + for (const [label, prelude] of spellings) { + const source = `${prelude}, { recursive: true, force: true })` + expect(findRawRecursiveRemovals(source), `${label} slipped past the scan`).toEqual([2]) + } + }) + + it('still exempts the retrying spellings and single-file removals', () => { + // The widened matcher must not start reporting the calls the rule is asking people to write. + expect( + findRawRecursiveRemovals( + "import * as fsp from 'node:fs/promises'\nawait fsp.rm(dir, { recursive: true, maxRetries: 8 })" + ) + ).toEqual([]) + expect( + findRawRecursiveRemovals( + "import { rm as removeDir } from 'node:fs/promises'\nawait removeDir(file, { force: true })" + ) + ).toEqual([]) + // `rm` inside a longer identifier is not a removal call. + expect(findRawRecursiveRemovals('confirmRemoval(dir, { recursive: true })')).toEqual([]) + }) + + it('removes trees through the retrying helper, never a raw recursive rm', () => { + const offenders = specs.flatMap((spec) => { + const source = readFileSync(join(REPO_ROOT, spec), 'utf8') + return findRawRecursiveRemovals(source).map((line) => `${spec}:${line}`) + }) + + expect( + offenders, + 'these teardowns can throw EPERM on Windows after their assertions have passed; use removeTree/removeTreeSync from src/shared/windows-transient-lock-removal.ts' + ).toEqual([]) + }) +}) diff --git a/src/shared/windows-transient-lock-removal.test.ts b/src/shared/windows-transient-lock-removal.test.ts new file mode 100644 index 00000000000..32b527ebfe6 --- /dev/null +++ b/src/shared/windows-transient-lock-removal.test.ts @@ -0,0 +1,117 @@ +import type * as NodeFs from 'node:fs' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { scanSourceTree, stripComments } from './source-scan/source-tree-scan' +import { + WINDOWS_RM_MAX_RETRIES, + WINDOWS_RM_RETRY_DELAY_MS, + removeTreeSync, + transientLockRemovalOptions +} from './windows-transient-lock-removal' + +const { rmSyncMock } = vi.hoisted(() => ({ + rmSyncMock: vi.fn() +})) + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, rmSync: rmSyncMock } +}) + +function withPlatform(platform: NodeJS.Platform): void { + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) +} + +const SOURCE_ROOT = join(__dirname, '..') +const OWNING_MODULE = 'shared/windows-transient-lock-removal.ts' +/** A `const WINDOWS_RM_… =` line, i.e. a file stating the policy rather than importing it. */ +const POLICY_DECLARATION = + /^\s*(?:export\s+)?const\s+WINDOWS_RM_(?:MAX_RETRIES|RETRY_DELAY_MS)\s*=/m + +/** Every file that declares the retry policy instead of importing it. */ +function findPolicyDeclarations(): string[] { + return scanSourceTree(SOURCE_ROOT, { includeTests: true }) + .filter( + (file) => + POLICY_DECLARATION.test(file.source) && POLICY_DECLARATION.test(stripComments(file.source)) + ) + .map((file) => file.relativePath) + .sort() +} + +describe('transient lock removal options', () => { + const originalPlatform = process.platform + + afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + rmSyncMock.mockReset() + }) + + it('retries on Windows, where a late handle release is the whole problem', () => { + withPlatform('win32') + + expect(transientLockRemovalOptions()).toEqual({ + recursive: true, + force: true, + maxRetries: WINDOWS_RM_MAX_RETRIES, + retryDelay: WINDOWS_RM_RETRY_DELAY_MS + }) + }) + + it('matches the repo policy of eight attempts', () => { + expect(WINDOWS_RM_MAX_RETRIES).toBe(8) + }) + + it('asks for no retries where removal is not raced by the OS', () => { + for (const platform of ['darwin', 'linux'] as const) { + withPlatform(platform) + expect(transientLockRemovalOptions()).toEqual({ recursive: true, force: true }) + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + } + }) + + it('retries a transient EPERM instead of treating force: true as enough', () => { + withPlatform('win32') + const eperm = Object.assign(new Error('EPERM: operation not permitted, unlink'), { + code: 'EPERM' + }) + rmSyncMock.mockImplementationOnce(() => { + throw eperm + }) + rmSyncMock.mockImplementationOnce(() => undefined) + + expect(() => removeTreeSync('C:\\temp\\orca-host-job')).not.toThrow() + expect(rmSyncMock).toHaveBeenCalledTimes(2) + expect(rmSyncMock.mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ recursive: true, force: true, maxRetries: WINDOWS_RM_MAX_RETRIES }) + ) + }) + + it('does not hide a non-lock removal failure', () => { + withPlatform('win32') + rmSyncMock.mockImplementation(() => { + throw Object.assign(new Error('EIO: i/o error'), { code: 'EIO' }) + }) + + expect(() => removeTreeSync('C:\\temp\\orca-host-job')).toThrow('EIO') + expect(rmSyncMock).toHaveBeenCalledTimes(1) + }) + + it('actually detects a file that states the policy', () => { + // Without this the scan below passes for any reason at all, including not scanning. + expect(POLICY_DECLARATION.test('const WINDOWS_RM_MAX_RETRIES = 8')).toBe(true) + expect(POLICY_DECLARATION.test(' export const WINDOWS_RM_RETRY_DELAY_MS = 150')).toBe(true) + // Importing the policy is the thing this rule is asking for, not a violation of it. + expect(POLICY_DECLARATION.test('import { WINDOWS_RM_MAX_RETRIES } from x')).toBe(false) + expect(POLICY_DECLARATION.test(' retryDelay: WINDOWS_RM_RETRY_DELAY_MS')).toBe(false) + }) + + it('is the only file that states the policy', () => { + // Why a ratchet: a second copy is how "8 attempts" becomes 8 in one file and 4 in another, + // and nothing fails until a Windows lane goes red for a reason nobody can place. + expect( + findPolicyDeclarations(), + 'declare the retry policy once, in src/shared/windows-transient-lock-removal.ts, and import it' + ).toEqual([OWNING_MODULE]) + }) +}) diff --git a/src/shared/windows-transient-lock-removal.ts b/src/shared/windows-transient-lock-removal.ts new file mode 100644 index 00000000000..e77eeecf7f2 --- /dev/null +++ b/src/shared/windows-transient-lock-removal.ts @@ -0,0 +1,64 @@ +// Why: Windows releases handles late. Antivirus, the search indexer, a just-exited child and a +// freshly dlopen'd DLL all keep a tree Node has just emptied locked for a few milliseconds, which +// surfaces as EBUSY/ENOTEMPTY/EPERM. Node's own `maxRetries` absorbs exactly that, and the repo +// already settled on 8 attempts — but only product code was using it, so test teardown kept +// failing tests whose assertions had already passed. + +import type { RmOptions } from 'node:fs' +import { rmSync } from 'node:fs' +import { rm } from 'node:fs/promises' + +export const WINDOWS_RM_MAX_RETRIES = 8 +export const WINDOWS_RM_RETRY_DELAY_MS = 150 + +/** `rm`/`rmSync` options for a recursive removal that must survive a late handle release. */ +export function transientLockRemovalOptions(): RmOptions { + const base = { recursive: true, force: true } + if (process.platform !== 'win32') { + return base + } + return { ...base, maxRetries: WINDOWS_RM_MAX_RETRIES, retryDelay: WINDOWS_RM_RETRY_DELAY_MS } +} + +function isTransientWindowsLockError(error: unknown): boolean { + if (process.platform !== 'win32' || typeof error !== 'object' || error === null) { + return false + } + const code = 'code' in error && typeof error.code === 'string' ? error.code : undefined + if (code && ['EBUSY', 'ENOTEMPTY', 'EPERM'].includes(code)) { + return true + } + const message = 'message' in error && typeof error.message === 'string' ? error.message : '' + return /directory not empty|resource busy|operation not permitted/i.test(message) +} + +function sleepSync(ms: number): void { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms) +} + +/** Recursively remove a directory, retrying the transient Windows locks. */ +export function removeTreeSync(targetPath: string): void { + const options = transientLockRemovalOptions() + const extraAttempts = process.platform === 'win32' ? WINDOWS_RM_MAX_RETRIES : 0 + let attempt = 0 + for (;;) { + try { + rmSync(targetPath, options) + return + } catch (error) { + // Why the outer loop: Node's `maxRetries` only runs inside a real `rmSync`. A mock, or a + // handle that outlives those inner attempts, still surfaces EPERM. `force: true` only + // suppresses ENOENT. + if (attempt >= extraAttempts || !isTransientWindowsLockError(error)) { + throw error + } + sleepSync(WINDOWS_RM_RETRY_DELAY_MS) + attempt += 1 + } + } +} + +/** Recursively remove a directory, retrying the transient Windows locks. */ +export async function removeTree(targetPath: string): Promise { + await rm(targetPath, transientLockRemovalOptions()) +} diff --git a/tests/e2e/helpers/client-hosted-browser-fixture.ts b/tests/e2e/helpers/client-hosted-browser-fixture.ts index ceafdde34e1..24ae9ab8816 100644 --- a/tests/e2e/helpers/client-hosted-browser-fixture.ts +++ b/tests/e2e/helpers/client-hosted-browser-fixture.ts @@ -2,6 +2,7 @@ import { createServer } from 'node:http' import type { AddressInfo } from 'node:net' import type { Page } from '@stablyai/playwright-test' import { expect } from './orca-app' +import { readRestartRendererState } from './orca-restart' import type { PairedElectronClient } from './paired-electron-client' /** @@ -64,13 +65,15 @@ export type MirroredBrowserPage = { } export async function findPairedWorktreeId(page: Page, repoPath: string): Promise { - return page.evaluate( - (path) => - window.__store - ?.getState() - .allWorktrees() - .find((worktree) => worktree.path === path)?.id ?? null, - repoPath + return readRestartRendererState(() => + page.evaluate( + (path) => + window.__store + ?.getState() + .allWorktrees() + .find((worktree) => worktree.path === path)?.id ?? null, + repoPath + ) ) } @@ -96,13 +99,15 @@ export async function selectPairedWorktreeGroup( await expect .poll( () => - page.evaluate( - ({ environmentId, worktreeId }) => { - const state = window.__store?.getState() - state?.setActiveWorktree(worktreeId, `runtime:${environmentId}`) - return state?.activeGroupIdByWorktree[worktreeId] ?? null - }, - { environmentId, worktreeId } + readRestartRendererState(() => + page.evaluate( + ({ environmentId, worktreeId }) => { + const state = window.__store?.getState() + state?.setActiveWorktree(worktreeId, `runtime:${environmentId}`) + return state?.activeGroupIdByWorktree[worktreeId] ?? null + }, + { environmentId, worktreeId } + ) ), { timeout: 120_000, message: 'paired client never activated a tab group for the worktree' } ) @@ -114,31 +119,33 @@ export async function findMirroredBrowserPage( worktreeId: string, url: string ): Promise { - return page.evaluate( - ({ url, worktreeId }) => { - const state = window.__store?.getState() - for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { - for (const browserPage of state?.browserPagesByWorkspace[workspace.id] ?? []) { - if (!browserPage.url.startsWith(url)) { - continue - } - const handle = state?.remoteBrowserPageHandlesByPageId[browserPage.id] - const visibleTab = (state?.unifiedTabsByWorktree[worktreeId] ?? []).find( - (tab) => tab.contentType === 'browser' && tab.entityId === workspace.id - ) - return { - localPageId: browserPage.id, - placementKind: handle?.placement?.kind ?? null, - remotePageId: handle?.remotePageId ?? browserPage.id, - url: browserPage.url, - visibleTabId: visibleTab?.id ?? null, - workspaceId: workspace.id + return readRestartRendererState(() => + page.evaluate( + ({ url, worktreeId }) => { + const state = window.__store?.getState() + for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { + for (const browserPage of state?.browserPagesByWorkspace[workspace.id] ?? []) { + if (!browserPage.url.startsWith(url)) { + continue + } + const handle = state?.remoteBrowserPageHandlesByPageId[browserPage.id] + const visibleTab = (state?.unifiedTabsByWorktree[worktreeId] ?? []).find( + (tab) => tab.contentType === 'browser' && tab.entityId === workspace.id + ) + return { + localPageId: browserPage.id, + placementKind: handle?.placement?.kind ?? null, + remotePageId: handle?.remotePageId ?? browserPage.id, + url: browserPage.url, + visibleTabId: visibleTab?.id ?? null, + workspaceId: workspace.id + } } } - } - return null - }, - { url, worktreeId } + return null + }, + { url, worktreeId } + ) ) } @@ -147,21 +154,25 @@ export async function readClientBrowserRows( page: Page, worktreeId: string ): Promise<{ pageId: string; placementKind: string | null; url: string }[]> { - return page.evaluate((worktreeId) => { - const state = window.__store?.getState() - const rows: { pageId: string; placementKind: string | null; url: string }[] = [] - for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { - for (const browserPage of state?.browserPagesByWorkspace[workspace.id] ?? []) { - rows.push({ - pageId: browserPage.id, - placementKind: - state?.remoteBrowserPageHandlesByPageId[browserPage.id]?.placement?.kind ?? null, - url: browserPage.url - }) - } - } - return rows - }, worktreeId) + return ( + (await readRestartRendererState(() => + page.evaluate((worktreeId) => { + const state = window.__store?.getState() + const rows: { pageId: string; placementKind: string | null; url: string }[] = [] + for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { + for (const browserPage of state?.browserPagesByWorkspace[workspace.id] ?? []) { + rows.push({ + pageId: browserPage.id, + placementKind: + state?.remoteBrowserPageHandlesByPageId[browserPage.id]?.placement?.kind ?? null, + url: browserPage.url + }) + } + } + return rows + }, worktreeId) + )) ?? [] + ) } export async function openClientHostedFixturePage( @@ -234,25 +245,27 @@ export async function readClientWebviewMarker( page: Page, target: { urlPrefix: string; remotePageId: string } ): Promise { - return page.evaluate(async ({ urlPrefix, remotePageId }) => { - const host = document.querySelector( - `[data-browser-client-page-id="${CSS.escape(remotePageId)}"]` - ) - for (const candidate of host?.querySelectorAll('webview') ?? []) { - const webview = candidate as Electron.WebviewTag - try { - if (!webview.getURL().startsWith(urlPrefix)) { - continue + return readRestartRendererState(() => + page.evaluate(async ({ urlPrefix, remotePageId }) => { + const host = document.querySelector( + `[data-browser-client-page-id="${CSS.escape(remotePageId)}"]` + ) + for (const candidate of host?.querySelectorAll('webview') ?? []) { + const webview = candidate as Electron.WebviewTag + try { + if (!webview.getURL().startsWith(urlPrefix)) { + continue + } + return (await webview.executeJavaScript( + 'document.querySelector("#marker")?.textContent ?? null' + )) as string | null + } catch { + // The guest may still be attaching. } - return (await webview.executeJavaScript( - 'document.querySelector("#marker")?.textContent ?? null' - )) as string | null - } catch { - // The guest may still be attaching. } - } - return null - }, target) + return null + }, target) + ) } export async function waitForRenderedClientWebview( @@ -289,8 +302,8 @@ export async function focusClientBrowserRow( export async function refreshAuthorityRuntimeId( client: PairedElectronClient ): Promise { - return client.page - .evaluate(async (environmentId) => { + return readRestartRendererState(() => + client.page.evaluate(async (environmentId) => { await window.api.runtimeEnvironments.connect({ selector: environmentId }) await window.__store?.getState().refreshRuntimeEnvironmentStatus(environmentId) return ( @@ -298,7 +311,7 @@ export async function refreshAuthorityRuntimeId( ?.runtimeId ?? null ) }, client.environmentId) - .catch(() => null) + ) } /** Waits until the client is talking to a genuinely new runtime process, not the one it paired to. */ diff --git a/tests/e2e/helpers/client-hosted-browser-fixture.unit.test.ts b/tests/e2e/helpers/client-hosted-browser-fixture.unit.test.ts new file mode 100644 index 00000000000..57aeec9dc0d --- /dev/null +++ b/tests/e2e/helpers/client-hosted-browser-fixture.unit.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest' +import type { Page } from '@stablyai/playwright-test' +import { + findMirroredBrowserPage, + findPairedWorktreeId, + readClientWebviewMarker +} from './client-hosted-browser-fixture' + +function pageWhoseEvaluateThrows(message: string): Page { + return { + evaluate: async () => { + throw new Error(message) + } + } as unknown as Page +} + +describe('client-hosted restart evaluate polling', () => { + it('treats a destroyed renderer context as a pending poll miss', async () => { + const page = pageWhoseEvaluateThrows( + 'Execution context was destroyed, most likely because of a navigation.' + ) + + await expect(findPairedWorktreeId(page, '/repo')).resolves.toBeNull() + await expect(findMirroredBrowserPage(page, 'wt-1', 'http://127.0.0.1/')).resolves.toBeNull() + await expect( + readClientWebviewMarker(page, { urlPrefix: 'http://127.0.0.1/', remotePageId: 'page-1' }) + ).resolves.toBeNull() + }) + + it('does not hide unrelated evaluate failures', async () => { + const page = pageWhoseEvaluateThrows('fetchWorktrees failed') + + await expect(findPairedWorktreeId(page, '/repo')).rejects.toThrow('fetchWorktrees failed') + await expect(findMirroredBrowserPage(page, 'wt-1', 'http://127.0.0.1/')).rejects.toThrow( + 'fetchWorktrees failed' + ) + await expect( + readClientWebviewMarker(page, { urlPrefix: 'http://127.0.0.1/', remotePageId: 'page-1' }) + ).rejects.toThrow('fetchWorktrees failed') + }) +}) diff --git a/tests/e2e/paired-client-hosted-browser-restart-survival.spec.ts b/tests/e2e/paired-client-hosted-browser-restart-survival.spec.ts index 95a8d1300ef..a9ece49a020 100644 --- a/tests/e2e/paired-client-hosted-browser-restart-survival.spec.ts +++ b/tests/e2e/paired-client-hosted-browser-restart-survival.spec.ts @@ -1,6 +1,3 @@ -import { createServer } from 'node:http' -import type { AddressInfo } from 'node:net' -import type { Page } from '@stablyai/playwright-test' import { expect, test } from './helpers/orca-app' import { launchHeadlessPairedRuntimeHost } from './helpers/headless-paired-runtime-host' import { readHostBrowserPageIds, readHostBrowserPageUrl } from './helpers/host-session-tabs' @@ -9,309 +6,22 @@ import { launchPairedElectronClient, type PairedElectronClient } from './helpers/paired-electron-client' +import { + findMirroredBrowserPage, + focusClientBrowserRow, + navigateGuest, + openClientHostedFixturePage, + readClientBrowserRows, + refreshAuthorityRuntimeId, + selectPairedWorktreeGroup, + startClientHostedMarkerFixture, + waitForPairedWorktreeId, + waitForRelaunchedRuntime, + waitForRenderedClientWebview +} from './helpers/client-hosted-browser-fixture' const CLIENT_NAME = 'STA-4150 client-hosted restart survival' -type MarkerFixture = { - close(): Promise - markerUrl: string - /** A second page the guest reaches on its own, to tell "survived" from "survived where". */ - movedUrl: string - origin: string -} - -async function startMarkerFixture(): Promise { - const server = createServer((request, response) => { - const marker = request.url === '/moved' ? 'moved-on' : 'restart-survivor' - response.writeHead(200, { - 'cache-control': 'no-store', - 'content-type': 'text/html; charset=utf-8' - }) - response.end( - `${marker}` + - `

${marker}

` - ) - }) - await new Promise((resolve, reject) => { - server.once('error', reject) - server.listen(0, '127.0.0.1', () => { - server.off('error', reject) - resolve() - }) - }) - const origin = `http://127.0.0.1:${(server.address() as AddressInfo).port}` - return { - close: () => - new Promise((resolve, reject) => { - server.closeAllConnections() - server.close((error) => (error ? reject(error) : resolve())) - }), - markerUrl: `${origin}/survivor`, - movedUrl: `${origin}/moved`, - origin - } -} - -/** Navigates the guest itself, the way following a link does — no client-side URL entry involved. */ -async function navigateGuest(page: Page, fromUrl: string, toUrl: string): Promise { - const navigated = await page.evaluate( - async ({ fromUrl, toUrl }) => { - for (const candidate of document.querySelectorAll('webview')) { - const webview = candidate as Electron.WebviewTag - try { - if (!webview.getURL().startsWith(fromUrl)) { - continue - } - await webview.loadURL(toUrl) - return true - } catch { - // The guest may still be attaching. - } - } - return false - }, - { fromUrl, toUrl } - ) - if (!navigated) { - throw new Error(`No client-hosted guest was showing ${fromUrl} to navigate`) - } -} - -type MirroredBrowserPage = { - localPageId: string - placementKind: 'client' | 'server' | null - remotePageId: string - url: string -} - -async function findPairedWorktreeId(page: Page, repoPath: string): Promise { - return page.evaluate( - (path) => - window.__store - ?.getState() - .allWorktrees() - .find((worktree) => worktree.path === path)?.id ?? null, - repoPath - ) -} - -async function waitForPairedWorktreeId(page: Page, repoPath: string): Promise { - await expect - .poll(() => findPairedWorktreeId(page, repoPath), { - timeout: 120_000, - message: 'paired client never received the host worktree' - }) - .not.toBeNull() - const worktreeId = await findPairedWorktreeId(page, repoPath) - if (!worktreeId) { - throw new Error('Paired worktree disappeared after discovery') - } - return worktreeId -} - -async function selectPairedWorktreeGroup( - page: Page, - environmentId: string, - worktreeId: string -): Promise { - await expect - .poll( - () => - page.evaluate( - ({ environmentId, worktreeId }) => { - const state = window.__store?.getState() - state?.setActiveWorktree(worktreeId, `runtime:${environmentId}`) - return state?.activeGroupIdByWorktree[worktreeId] ?? null - }, - { environmentId, worktreeId } - ), - { - timeout: 120_000, - message: 'paired client never activated a tab group for the worktree' - } - ) - .not.toBeNull() -} - -async function findMirroredBrowserPage( - page: Page, - worktreeId: string, - url: string -): Promise { - return page.evaluate( - ({ url, worktreeId }) => { - const state = window.__store?.getState() - for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { - for (const browserPage of state?.browserPagesByWorkspace[workspace.id] ?? []) { - if (!browserPage.url.startsWith(url)) { - continue - } - const handle = state?.remoteBrowserPageHandlesByPageId[browserPage.id] - return { - localPageId: browserPage.id, - placementKind: handle?.placement?.kind ?? null, - remotePageId: handle?.remotePageId ?? browserPage.id, - url: browserPage.url - } - } - } - return null - }, - { url, worktreeId } - ) -} - -/** Every browser row the client holds for a worktree, for diagnosing duplicates and culls. */ -async function readClientBrowserRows( - page: Page, - worktreeId: string -): Promise<{ pageId: string; placementKind: string | null; url: string }[]> { - return page.evaluate((worktreeId) => { - const state = window.__store?.getState() - const rows: { pageId: string; placementKind: string | null; url: string }[] = [] - for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { - for (const browserPage of state?.browserPagesByWorkspace[workspace.id] ?? []) { - rows.push({ - pageId: browserPage.id, - placementKind: - state?.remoteBrowserPageHandlesByPageId[browserPage.id]?.placement?.kind ?? null, - url: browserPage.url - }) - } - } - return rows - }, worktreeId) -} - -async function createProductBrowserPage(page: Page, url: string): Promise { - await page.evaluate(async (url) => { - const state = window.__store?.getState() - if (!state?.activeWorktreeId) { - throw new Error('Paired client has no active worktree') - } - const groupId = state.activeGroupIdByWorktree[state.activeWorktreeId] - if (!groupId) { - throw new Error('Paired client has no active tab group') - } - state.setBrowserDefaultUrl(url) - await state.openNewBrowserTabInActiveWorkspace(groupId) - }, url) -} - -async function openClientHostedFixturePage( - client: PairedElectronClient, - worktreeId: string, - url: string -): Promise { - await createProductBrowserPage(client.page, url) - await expect - .poll(() => findMirroredBrowserPage(client.page, worktreeId, url), { - timeout: 60_000, - message: `paired client never materialized ${url}` - }) - .not.toBeNull() - const mirrored = await findMirroredBrowserPage(client.page, worktreeId, url) - if (!mirrored) { - throw new Error(`Mirrored browser page disappeared for ${url}`) - } - expect(mirrored.placementKind, 'fixture page must be hosted on the viewing desktop').toBe( - 'client' - ) - await focusClientBrowserRow(client.page, worktreeId, mirrored.localPageId) - return mirrored -} - -/** - * Reads the marker out of the guest belonging to one specific page. - * - * Bound to that page's retained host rather than scanning every ``: a scan by URL alone is - * satisfied by any guest on the fixture origin, so a run that lost the surviving tab and opened a - * fresh one would still read `moved-on` and pass. Client-hosted guests never enter their pane's - * subtree -- the host is a fixed-position overlay -- so the binding is the stamped page id, which - * is also the identity the restart has to preserve. - */ -async function readClientWebviewMarker( - page: Page, - target: { urlPrefix: string; remotePageId: string } -): Promise { - return page.evaluate(async ({ urlPrefix, remotePageId }) => { - const host = document.querySelector( - `[data-browser-client-page-id="${CSS.escape(remotePageId)}"]` - ) - for (const candidate of host?.querySelectorAll('webview') ?? []) { - const webview = candidate as Electron.WebviewTag - try { - if (!webview.getURL().startsWith(urlPrefix)) { - continue - } - return (await webview.executeJavaScript( - 'document.querySelector("#marker")?.textContent ?? null' - )) as string | null - } catch { - // The guest may still be attaching. - } - } - return null - }, target) -} - -async function waitForRenderedClientWebview( - page: Page, - target: { urlPrefix: string; remotePageId: string }, - message: string -): Promise { - await expect - .poll(() => readClientWebviewMarker(page, target), { timeout: 120_000, message }) - .not.toBeNull() - const marker = await readClientWebviewMarker(page, target) - if (!marker) { - throw new Error(`Client-hosted guest for ${target.urlPrefix} lost its marker`) - } - return marker -} - -/** Surfaces a row's pane so its guest is mounted where the scoped marker read can see it. */ -async function focusClientBrowserRow( - page: Page, - worktreeId: string, - localPageId: string -): Promise { - await page.evaluate( - ({ browserPageId, worktreeId }) => { - window.__store?.getState().focusBrowserTabInWorktree(worktreeId, browserPageId, { - surfacePane: true - }) - }, - { browserPageId: localPageId, worktreeId } - ) -} - -async function refreshAuthorityRuntimeId(client: PairedElectronClient): Promise { - return client.page - .evaluate(async (environmentId) => { - await window.api.runtimeEnvironments.connect({ selector: environmentId }) - await window.__store?.getState().refreshRuntimeEnvironmentStatus(environmentId) - return ( - window.__store?.getState().runtimeStatusByEnvironmentId.get(environmentId)?.status - ?.runtimeId ?? null - ) - }, client.environmentId) - .catch(() => null) -} - -/** Waits until the client is talking to a genuinely new runtime process, not the one it paired to. */ -async function waitForRelaunchedRuntime( - client: PairedElectronClient, - previousRuntimeId: string -): Promise { - await expect - .poll(() => refreshAuthorityRuntimeId(client), { - timeout: 180_000, - message: 'paired client never reconnected to a relaunched runtime process' - }) - .toEqual(expect.not.stringMatching(`^${previousRuntimeId}$`)) -} - /** * Server-restart half of the tab-persistence contract. The client-quit half is covered by * paired-client-hosted-browser-quit-survival.spec.ts. @@ -334,7 +44,10 @@ test('keeps a client-hosted browser tab across a paired runtime restart', async testRepoPath }, testInfo) => { test.setTimeout(420_000) - const fixture = await startMarkerFixture() + const fixture = await startClientHostedMarkerFixture({ + created: 'restart-survivor', + moved: 'moved-on' + }) const host = await launchHeadlessPairedRuntimeHost({ pinnedServePort: true }) let client: PairedElectronClient | null = null try {