mirror of
https://github.com/stablyai/orca.git
synced 2026-10-09 00:02:39 +00:00
fix(startup): Windows never crashes resolving userData when roaming AppData is unavailable (#25113)
* fix(startup): pin Windows appData and userData before anything resolves them A Windows session without a loaded profile (e.g. orca serve over SSH) can fail the roaming AppData known-folder lookup. Electron 43 then falls through to Chromium's userData provider and crashes natively. Resolve appData first (falling back to APPDATA, then USERPROFILE\AppData\Roaming), and set userData explicitly so Electron's provider never runs. * test(startup): remove the AppData fixture through the retrying helper --------- Co-authored-by: m4air <m4air@Mac.localdomain>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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'],
|
||||
|
||||
@@ -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' },
|
||||
|
||||
@@ -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'
|
||||
])
|
||||
})
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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<string, string>
|
||||
} {
|
||||
const calls: string[] = []
|
||||
const paths = new Map<string, string>()
|
||||
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)
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -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()))
|
||||
}
|
||||
@@ -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 })
|
||||
}
|
||||
})
|
||||
Reference in New Issue
Block a user