mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
fix(startup): install the app environment with the userData decision (#17755)
src/main/index.ts decided where userData lives at module scope, then installed the AppEnvironment port ~180 lines later inside the single-instance-lock block. Every statement in that gap was a latent failure: a path resolve there either threw 'AppEnvironment not initialized' and killed the process, or — with the accessor installed but the decision not yet run — would have memoized the pre-override directory in getCanonicalUserDataPath() for the whole session. The first outcome shipped. #16761/#16698/#17509 were one statement landing in that gap and killing every macOS `orca serve` across 1.4.190-1.4.192; #16762 moved that call but left the gap. Install the port and capture the canonical path immediately after the two calls that decide them, so the window is zero rather than small. Both are inert at this point — ElectronAppEnvironment holds no state and calls `app` lazily per accessor, and initDataPath only joins strings — so nothing that depended on the old position moves with them. The secret store stays where its pre-ready Keychain note applies. The throw is kept and still covers the case it should: resolving a path before the decision has run. Guarded by a source-level assertion that the decision, the install and the capture stay adjacent. Fixes #17750
This commit is contained in:
+15
-8
@@ -762,6 +762,16 @@ if (app.isPackaged && process.platform !== 'win32') {
|
||||
}
|
||||
configureDevUserDataPath(is.dev)
|
||||
configureOrcaUserDataPathEnv()
|
||||
// Why these four lines are one step (#16761): the two above decide where userData lives, and
|
||||
// everything below may resolve a path. Installing the accessor any later leaves a window where an
|
||||
// early resolve either throws — which is what killed `orca serve` — or, worse, memoizes the
|
||||
// pre-override directory and silently writes user state to the wrong place for the whole session.
|
||||
// Safe this early: ElectronAppEnvironment holds no state and calls `app` lazily per accessor, so it
|
||||
// changes no timing, and initDataPath only joins strings.
|
||||
setAppEnvironment(new ElectronAppEnvironment())
|
||||
// Why captured now: after the dev/E2E override above, and before app.setName('Orca') (whenReady)
|
||||
// changes how userData resolves on a case-sensitive filesystem. See persistence.ts:20-28.
|
||||
initDataPath()
|
||||
|
||||
// Why: just past createMainWindow's 10s ready-to-show fallback, so a window revealed that way still gets its tray icon.
|
||||
const TRAY_CREATE_FALLBACK_MS = 12_000
|
||||
@@ -936,12 +946,11 @@ if (!hasSingleInstanceLock) {
|
||||
|
||||
// Why: when another process holds the lock we've already exited; skip file-writing side effects so this transient process never touches userData.
|
||||
if (hasSingleInstanceLock) {
|
||||
// Why first: both accessors throw until installed, and everything below this line
|
||||
// may resolve a path or read a credential. Neither constructor touches `app` or
|
||||
// `safeStorage` — they resolve lazily per call — so installing here changes no
|
||||
// timing, in particular not the pre-ready Keychain service-name resolution and
|
||||
// the app.setName ordering the userData captures below depend on.
|
||||
setAppEnvironment(new ElectronAppEnvironment())
|
||||
// Why first in this block: the accessor throws until installed and everything below may read a
|
||||
// credential. The constructor does not touch `safeStorage` — it resolves lazily per call — so
|
||||
// installing here changes no timing, in particular not the pre-ready Keychain service-name
|
||||
// resolution. The app-environment port and the userData capture install earlier still, next to
|
||||
// the path decision they depend on.
|
||||
setSecretStore(new ElectronSecretStore())
|
||||
// Why at process level, not per-window: pty.ts registers against injected surfaces so
|
||||
// it can load without electron, and an Electron main process always has ipcMain —
|
||||
@@ -974,8 +983,6 @@ if (hasSingleInstanceLock) {
|
||||
installDevParentDisconnectQuit(shouldCoupleToDevParent)
|
||||
installDevParentWatchdog(shouldCoupleToDevParent)
|
||||
installDevParentSignalQuit(shouldCoupleToDevParent)
|
||||
// Why: run after configureDevUserDataPath but before app.setName('Orca') (whenReady), which changes the resolved path on case-sensitive filesystems.
|
||||
initDataPath()
|
||||
// Why not at module scope with the other lifetime couplings (#16761): this resolves the handoff
|
||||
// path, so it throws until setAppEnvironment() above installs the accessor — which killed every
|
||||
// `orca serve` process before it could listen. After initDataPath() specifically, so the
|
||||
|
||||
@@ -362,11 +362,14 @@ describe('startup ordering', () => {
|
||||
// leaves the serve process orphaned on its port, which is the failure this handler prevents.
|
||||
expect(installIndex).toBeLessThan(source.indexOf('void app.whenReady().then('))
|
||||
expect(installIndex).toBeGreaterThan(source.indexOf('if (hasSingleInstanceLock) {'))
|
||||
const betweenCode = source
|
||||
.slice(dataPathIndex, installIndex)
|
||||
// Why only statements at block indentation: the span now covers unrelated helper functions,
|
||||
// and an `await` inside one of those bodies is not what this guards against — the risk is this
|
||||
// call itself being parked behind one.
|
||||
const blockStatements = source
|
||||
.slice(source.indexOf('if (hasSingleInstanceLock) {'), installIndex)
|
||||
.split('\n')
|
||||
.filter((line) => !line.trim().startsWith('//'))
|
||||
.filter((line) => /^ {2}\S/.test(line) && !line.trim().startsWith('//'))
|
||||
.join('\n')
|
||||
expect(betweenCode).not.toContain('await')
|
||||
expect(blockStatements).not.toContain('await')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -52,6 +52,33 @@ describe('host port bootstrap wiring', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('installs the app environment as part of the userData decision, not after it', () => {
|
||||
// Why (#16761): the accessor throws until installed, and `getCanonicalUserDataPath()` memoizes
|
||||
// whatever it first resolves. Any gap between deciding where userData lives and installing the
|
||||
// port is a window where an early path resolve either kills the process — which is what took
|
||||
// down every macOS `orca serve` — or caches the pre-override directory for the whole session.
|
||||
// Keeping the four statements adjacent is what makes that window zero rather than merely small.
|
||||
const decide = source.indexOf('configureDevUserDataPath(is.dev)')
|
||||
const install = source.indexOf('setAppEnvironment(new ElectronAppEnvironment())')
|
||||
const capture = source.indexOf('initDataPath()')
|
||||
|
||||
expect(decide).toBeGreaterThanOrEqual(0)
|
||||
expect(install).toBeGreaterThan(decide)
|
||||
expect(capture).toBeGreaterThan(install)
|
||||
|
||||
const statements = source
|
||||
.slice(decide, capture)
|
||||
.split('\n')
|
||||
.map((line) => line.trim())
|
||||
.filter((line) => line.length > 0 && !line.startsWith('//'))
|
||||
|
||||
expect(statements).toEqual([
|
||||
'configureDevUserDataPath(is.dev)',
|
||||
'configureOrcaUserDataPathEnv()',
|
||||
'setAppEnvironment(new ElectronAppEnvironment())'
|
||||
])
|
||||
})
|
||||
|
||||
it('installs the ports at process level, not per window', () => {
|
||||
// Why: installing per window registered the PTY surfaces against no-ops on the
|
||||
// serve path, where no window ever opens. Caught in CI by the SSH docker E2E.
|
||||
|
||||
Reference in New Issue
Block a user