diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 62aa82b73f7..22f167feec7 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -342,7 +342,8 @@ jobs: . != "tests/e2e/ssh-localhost.spec.ts" and . != "tests/e2e/terminal-ibus-hangul-native.spec.ts" and . != "tests/e2e/orcad-serve-mode-switch.spec.ts" and - . != "tests/e2e/ssh-orcad-auto-convert.spec.ts" + . != "tests/e2e/ssh-orcad-auto-convert.spec.ts" and + . != "tests/e2e/windows-missing-appdata-startup.spec.ts" )' <<<"$TEST_FILES_JSON" > "$RUNNER_TEMP/general-e2e-specs" fi mapfile -t TEST_FILES < "$RUNNER_TEMP/general-e2e-specs" @@ -552,6 +553,44 @@ jobs: retention-days: 7 if-no-files-found: ignore + # A profile-less Windows session has no roaming AppData; Electron 43 used to crash natively there. + windows-missing-appdata-startup: + name: orca serve starts without AppData on Windows + if: inputs.test_files == '' || contains(inputs.test_files, 'tests/e2e/windows-missing-appdata-startup.spec.ts') + runs-on: windows-2022 + timeout-minutes: 30 + env: + NODE_OPTIONS: --max-old-space-size=4096 + ORCA_BACKGROUND_LAUNCH: '1' + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ inputs.ref || github.ref }} + persist-credentials: false + - uses: ./.github/actions/install-node-dependencies + with: + native-runtime: electron + - name: Build the e2e app and CLI + shell: bash + run: | + pnpm exec electron-vite build --mode e2e + pnpm run build:cli + - name: Start serve with no AppData folder + env: + SKIP_BUILD: '1' + ORCA_E2E_FORWARD_APP_LOGS: '1' + ORCA_STARTUP_DIAGNOSTICS: '1' + ELECTRON_ENABLE_LOGGING: '1' + ELECTRON_ENABLE_STACK_DUMPING: '1' + run: pnpm exec playwright test --config tests/playwright.config.ts tests/e2e/windows-missing-appdata-startup.spec.ts --project=electron-headless --workers=1 + - uses: actions/upload-artifact@v7 + if: failure() + with: + name: windows-missing-appdata-startup-traces + path: test-results/ + retention-days: 7 + if-no-files-found: ignore + # #24979 on a real host: a relay-era Docker host converts to managed orcad on connect. Needs the # orcad template for the fixture's target (Debian, linux-x64-glibc), which only this job builds. orcad-auto-convert-docker: diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0cbb0f2087f..cdebe9c7ebb 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -1154,6 +1154,7 @@ jobs: src/main/cursor/hook-service.test.ts src/main/orca-profiles/profile-index-store.test.ts src/main/startup/windows-install-dir-acl-repair.win32.test.ts + src/main/startup/windows-app-data-path.test.ts src/main/runtime/repo-worktree-admin-fingerprint.test.ts src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts src/shared/secure-file-fsync-flags.test.ts diff --git a/config/scripts/ci-e2e-job-selection.mjs b/config/scripts/ci-e2e-job-selection.mjs index 644fc6a5bc5..b31e68008eb 100644 --- a/config/scripts/ci-e2e-job-selection.mjs +++ b/config/scripts/ci-e2e-job-selection.mjs @@ -41,13 +41,16 @@ export const NATIVE_IME_E2E_SPEC = 'tests/e2e/terminal-ibus-hangul-native.spec.t export const ORCAD_SERVE_MODE_SWITCH_E2E_SPEC = 'tests/e2e/orcad-serve-mode-switch.spec.ts' // Needs the orcad template for its host's target, which only its own job builds. export const ORCAD_AUTO_CONVERT_E2E_SPEC = 'tests/e2e/ssh-orcad-auto-convert.spec.ts' +// Windows-only; its own job runs it on a Windows runner. +export const WINDOWS_MISSING_APPDATA_E2E_SPEC = 'tests/e2e/windows-missing-appdata-startup.spec.ts' export const DEDICATED_E2E_SPECS = [ ...DOCKER_SSH_E2E_SPECS, NODE_NETWORK_E2E_SPEC, LOCALHOST_SSH_E2E_SPEC, NATIVE_IME_E2E_SPEC, ORCAD_SERVE_MODE_SWITCH_E2E_SPEC, - ORCAD_AUTO_CONVERT_E2E_SPEC + ORCAD_AUTO_CONVERT_E2E_SPEC, + WINDOWS_MISSING_APPDATA_E2E_SPEC ] const dedicatedSpecs = new Set(DEDICATED_E2E_SPECS) const dockerSpecs = new Set(DOCKER_SSH_E2E_SPECS) diff --git a/config/scripts/pr-e2e-source-routing.mjs b/config/scripts/pr-e2e-source-routing.mjs index 9ee6998d76d..bab26f13fd0 100644 --- a/config/scripts/pr-e2e-source-routing.mjs +++ b/config/scripts/pr-e2e-source-routing.mjs @@ -22,6 +22,13 @@ export const PR_E2E_SOURCE_ROUTES = [ file ) }, + { + id: 'startup.windows-missing-appdata', + specs: ['tests/e2e/windows-missing-appdata-startup.spec.ts'], + matches: (file) => + isProductSource(file) && + /^src\/main\/startup\/(?:windows-app-data-path|main-process-preflight)\.ts$/.test(file) + }, { id: 'ssh.orcad-auto-convert', specs: ['tests/e2e/ssh-orcad-auto-convert.spec.ts'], diff --git a/config/scripts/release-cut-token-permissions.test.mjs b/config/scripts/release-cut-token-permissions.test.mjs index 7e6267af6d6..41d24c100af 100644 --- a/config/scripts/release-cut-token-permissions.test.mjs +++ b/config/scripts/release-cut-token-permissions.test.mjs @@ -17,6 +17,7 @@ const EXPECTED_MATRIX = { '.github/workflows/e2e.yml#ssh-browser-network-route': { contents: 'read' }, '.github/workflows/e2e.yml#ssh-localhost': { contents: 'read' }, '.github/workflows/e2e.yml#ssh-docker-watcher-isolation': { contents: 'read' }, + '.github/workflows/e2e.yml#windows-missing-appdata-startup': { contents: 'read' }, '.github/workflows/homebrew-bump.yml#bump-cask': { contents: 'read' }, '.github/workflows/node-server-tests.yml#changes': { contents: 'read' }, '.github/workflows/node-server-tests.yml#desktop_template': { contents: 'read' }, diff --git a/config/scripts/ssh-docker-ci-sharding.test.mjs b/config/scripts/ssh-docker-ci-sharding.test.mjs index facbc005d50..4afddbe6fed 100644 --- a/config/scripts/ssh-docker-ci-sharding.test.mjs +++ b/config/scripts/ssh-docker-ci-sharding.test.mjs @@ -37,7 +37,8 @@ it('gives every removed SSH spec a dedicated owner even for test-only edits', () 'tests/e2e/ssh-browser-network-execution-route.docker.unit.test.ts', 'tests/e2e/ssh-localhost.spec.ts', 'tests/e2e/ssh-orcad-auto-convert.spec.ts', - 'tests/e2e/terminal-ibus-hangul-native.spec.ts' + 'tests/e2e/terminal-ibus-hangul-native.spec.ts', + 'tests/e2e/windows-missing-appdata-startup.spec.ts' ]) }) diff --git a/src/main/startup/main-process-preflight.ts b/src/main/startup/main-process-preflight.ts index 337d01e464d..e16db687311 100644 --- a/src/main/startup/main-process-preflight.ts +++ b/src/main/startup/main-process-preflight.ts @@ -107,6 +107,7 @@ import { initializeBrowserIdentityModeStore } from '../browser/browser-identity- import { acquireProfileStateRuntimeAdmission } from '../persistence/profile-state/profile-state-access' import { getActiveProfileStateLocation } from '../persistence/profile-state/profile-state-active-location' import { handleMainProcessPreflightFailure } from './main-process-preflight-failure' +import { ensureWindowsAppDataPath } from './windows-app-data-path' export type MainProcessPreflightOptions = { focusExistingWindow: () => void @@ -125,6 +126,8 @@ export function runMainProcessPreflight(options: MainProcessPreflightOptions): b } function initializeMainProcessPreflight(options: MainProcessPreflightOptions): boolean { + // Why first: every step below, recovery and the instance lock included, may resolve userData. + ensureWindowsAppDataPath(app) if (runProfileStateRecoveryPreflight()) { return false } diff --git a/src/main/startup/windows-app-data-path.test.ts b/src/main/startup/windows-app-data-path.test.ts new file mode 100644 index 00000000000..8cbff4f39dd --- /dev/null +++ b/src/main/startup/windows-app-data-path.test.ts @@ -0,0 +1,135 @@ +import { existsSync, mkdtempSync, readFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, win32 } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { removeTreeSync } from '../../shared/windows-transient-lock-removal' +import { + deriveWindowsAppDataPath, + ensureWindowsAppDataPath, + type WindowsAppDataPathHost +} from './windows-app-data-path' + +const ERROR_MESSAGE = /could not find the Windows roaming AppData folder/ + +function createHost(nativeAppData: string | Error): WindowsAppDataPathHost & { + calls: string[] + paths: Map +} { + const calls: string[] = [] + const paths = new Map() + return { + calls, + paths, + getName: () => 'orca', + getPath: (name) => { + calls.push(`get:${name}`) + const overridden = paths.get(name) + if (overridden) { + return overridden + } + if (nativeAppData instanceof Error) { + throw nativeAppData + } + return nativeAppData + }, + setPath: (name, value) => { + calls.push(`set:${name}`) + paths.set(name, value) + } + } +} + +const tempDirs: string[] = [] +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + removeTreeSync(dir) + } +}) + +describe('deriveWindowsAppDataPath', () => { + it('prefers APPDATA', () => { + expect( + deriveWindowsAppDataPath({ + APPDATA: 'D:\\Profiles\\me\\Roaming', + USERPROFILE: 'C:\\Users\\me' + }) + ).toBe('D:\\Profiles\\me\\Roaming') + }) + + it('falls back to USERPROFILE\\AppData\\Roaming when APPDATA is unset or relative', () => { + expect(deriveWindowsAppDataPath({ USERPROFILE: 'C:\\Users\\me' })).toBe( + 'C:\\Users\\me\\AppData\\Roaming' + ) + expect(deriveWindowsAppDataPath({ APPDATA: 'Roaming', USERPROFILE: 'C:\\Users\\me' })).toBe( + 'C:\\Users\\me\\AppData\\Roaming' + ) + }) + + it('throws a readable error when neither variable is usable', () => { + expect(() => deriveWindowsAppDataPath({})).toThrow(ERROR_MESSAGE) + expect(() => deriveWindowsAppDataPath({ APPDATA: ' ', USERPROFILE: '' })).toThrow(ERROR_MESSAGE) + }) +}) + +describe('ensureWindowsAppDataPath', () => { + it('does nothing off Windows', () => { + const host = createHost(new Error('unused')) + ensureWindowsAppDataPath(host, {}, 'darwin') + expect(host.calls).toEqual([]) + }) + + it('keeps the native appData and pins userData to the path Electron would derive', () => { + const host = createHost('C:\\Users\\me\\AppData\\Roaming') + ensureWindowsAppDataPath(host, { APPDATA: 'D:\\ignored' }, 'win32') + expect(host.calls).toEqual(['get:appData', 'set:userData']) + expect(host.paths.get('userData')).toBe('C:\\Users\\me\\AppData\\Roaming\\orca') + }) + + it('creates and sets appData from the environment before userData when the lookup throws', () => { + // Why a real dir: the fallback must mkdir a folder that a profile-less session never created. + const root = mkdtempSync(join(tmpdir(), 'orca-appdata-')) + tempDirs.push(root) + const missingAppData = join(root, 'missing', 'Roaming') + const host = createHost(new Error('Failed to get appData path')) + + ensureWindowsAppDataPath(host, { APPDATA: missingAppData }, 'win32') + + expect(host.calls).toEqual(['get:appData', 'set:appData', 'set:userData']) + expect(host.paths.get('appData')).toBe(missingAppData) + expect(host.paths.get('userData')).toBe(win32.join(missingAppData, 'orca')) + expect(existsSync(missingAppData)).toBe(true) + }) + + it('treats an empty native appData like a failed lookup', () => { + const host = createHost('') + expect(() => ensureWindowsAppDataPath(host, {}, 'win32')).toThrow(ERROR_MESSAGE) + expect(host.calls).toEqual(['get:appData']) + }) + + it('fails with a readable error, setting nothing, when both variables are missing', () => { + const host = createHost(new Error('Failed to get appData path')) + expect(() => ensureWindowsAppDataPath(host, {}, 'win32')).toThrow(ERROR_MESSAGE) + expect(host.paths.size).toBe(0) + }) +}) + +describe('startup ordering', () => { + it('pins Windows appData before any preflight step can resolve userData', () => { + const source = readFileSync( + join(process.cwd(), 'src/main/startup/main-process-preflight.ts'), + 'utf8' + ) + const body = source.slice(source.indexOf('function initializeMainProcessPreflight(')) + const ensure = body.indexOf('ensureWindowsAppDataPath(app)') + expect(ensure).toBeGreaterThan(0) + for (const laterStep of [ + 'runProfileStateRecoveryPreflight()', + 'maybeRedirectCliLaunch(', + 'configureDevUserDataPath(', + 'configureOrcaUserDataPathEnv()', + 'startCrashpadCapture()' + ]) { + expect(body.indexOf(laterStep)).toBeGreaterThan(ensure) + } + }) +}) diff --git a/src/main/startup/windows-app-data-path.ts b/src/main/startup/windows-app-data-path.ts new file mode 100644 index 00000000000..04e29d2dfc7 --- /dev/null +++ b/src/main/startup/windows-app-data-path.ts @@ -0,0 +1,55 @@ +import { mkdirSync } from 'node:fs' +import { win32 } from 'node:path' + +export type WindowsAppDataPathHost = { + getPath(name: 'appData'): string + setPath(name: 'appData' | 'userData', value: string): void + getName(): string +} + +/** Derives the roaming AppData folder from the environment when the known-folder lookup fails. */ +export function deriveWindowsAppDataPath(env: NodeJS.ProcessEnv): string { + const appData = env.APPDATA?.trim() + if (appData && win32.isAbsolute(appData)) { + return appData + } + const userProfile = env.USERPROFILE?.trim() + if (userProfile && win32.isAbsolute(userProfile)) { + return win32.join(userProfile, 'AppData', 'Roaming') + } + throw new Error( + 'Orca could not find the Windows roaming AppData folder: Windows did not report it, and neither APPDATA nor USERPROFILE is set.' + ) +} + +function readNativeAppDataPath(host: WindowsAppDataPathHost): string | null { + try { + return host.getPath('appData') || null + } catch { + return null + } +} + +/** + * Pins appData and userData before anything resolves userData on Windows. + * + * Why: when Electron cannot resolve or create userData itself (no loaded profile, e.g. over SSH), it falls + * through to Chromium's provider, which dereferences null and crashes natively before crashpad connects. + */ +export function ensureWindowsAppDataPath( + host: WindowsAppDataPathHost, + env: NodeJS.ProcessEnv = process.env, + platform: NodeJS.Platform = process.platform +): void { + if (platform !== 'win32') { + return + } + let appData = readNativeAppDataPath(host) + if (!appData) { + appData = deriveWindowsAppDataPath(env) + mkdirSync(appData, { recursive: true }) + host.setPath('appData', appData) + } + // Same value Electron's own provider computes; setting it skips that provider entirely. + host.setPath('userData', win32.join(appData, host.getName())) +} diff --git a/tests/e2e/windows-missing-appdata-startup.spec.ts b/tests/e2e/windows-missing-appdata-startup.spec.ts new file mode 100644 index 00000000000..41bb7813b3e --- /dev/null +++ b/tests/e2e/windows-missing-appdata-startup.spec.ts @@ -0,0 +1,94 @@ +import { existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { _electron as electron, type ElectronApplication } from '@stablyai/playwright-test' +import { test, expect, forwardElectronProcessLogs } from './helpers/orca-app' +import { getE2ECompletedOnboardingProfile } from './helpers/e2e-completed-onboarding-profile' +import { getOrcaElectronLaunchArgs } from './helpers/electron-launch-args' +import { cleanupE2EDaemons, closeElectronAppForE2E } from './helpers/electron-process-shutdown' +import { + assertElectronResolvedIsolatedHome, + createElectronHomeIsolation +} from './helpers/electron-home-isolation' +import { RuntimeClient } from '../../src/cli/runtime/client' +import { RuntimeClientError } from '../../src/cli/runtime/types' + +// Regression: a profile-less Windows session (e.g. `orca serve` over SSH) has no roaming AppData, +// and Electron 43 crashed natively resolving userData before Orca pinned it. +test('orca serve starts on Windows when the home has no AppData folder', async (// oxlint-disable-next-line no-empty-pattern -- This spec owns its launch and opts out of the default app fixture. +{}, testInfo) => { + test.skip(process.platform !== 'win32', 'Windows AppData resolution only') + + const mainPath = path.join(process.cwd(), 'out', 'main', 'index.js') + const userDataDir = mkdtempSync(path.join(os.tmpdir(), 'orca-e2e-missing-appdata-')) + const missingAppData = path.join(userDataDir, 'absent', 'AppData', 'Roaming') + const { ELECTRON_RUN_AS_NODE: _unused, ...cleanEnv } = process.env + void _unused + const isolation = createElectronHomeIsolation({ + inheritedEnv: cleanEnv, + launchEnv: { + NODE_ENV: 'development', + ORCA_E2E_HEADLESS: '1', + APPDATA: missingAppData + }, + extraEnv: {}, + userDataDir + }) + rmSync(path.join(isolation.isolatedHome, 'AppData'), { recursive: true, force: true }) + expect(existsSync(missingAppData)).toBe(false) + writeFileSync( + path.join(userDataDir, 'orca-data.json'), + `${JSON.stringify(getE2ECompletedOnboardingProfile(), null, 2)}\n` + ) + + let serveApp: ElectronApplication | null = null + try { + serveApp = await electron.launch({ + args: [...getOrcaElectronLaunchArgs(mainPath, false), '--serve', '--serve-no-pairing'], + env: Object.fromEntries( + Object.entries(isolation.env).filter( + (entry): entry is [string, string] => entry[1] !== undefined + ) + ) + }) + forwardElectronProcessLogs(serveApp, testInfo) + const paths = await serveApp.evaluate(({ app }) => ({ + home: app.getPath('home'), + appData: app.getPath('appData'), + userData: app.getPath('userData') + })) + assertElectronResolvedIsolatedHome(paths.home, isolation) + // Why realpath: tmpdir can be an 8.3 short-name alias of the path Electron reports. + expect(realpathSync.native(paths.userData).toLowerCase()).toBe( + realpathSync.native(userDataDir).toLowerCase() + ) + // Records whether Windows reported AppData or Orca fell back to the environment. + testInfo.annotations.push({ + type: 'appData', + description: `${paths.appData === missingAppData ? 'env fallback' : 'native'}: ${paths.appData}` + }) + + const client = new RuntimeClient(userDataDir, 5_000) + await expect + .poll( + async () => { + try { + return (await client.getCliStatus()).result.app.desktopWindowStatus + } catch (error) { + if (error instanceof RuntimeClientError && error.code === 'runtime_unavailable') { + return 'starting' + } + throw error + } + }, + { timeout: 60_000, message: 'orca serve never became ready without AppData' } + ) + .toBe('openable') + } finally { + if (serveApp) { + await closeElectronAppForE2E(serveApp) + } + await cleanupE2EDaemons(userDataDir) + rmSync(userDataDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 }) + } +})