diff --git a/src/main/index.ts b/src/main/index.ts index f1cb0fd6cd8..ede6b0496e1 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -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 diff --git a/src/main/startup/desktop-startup-ordering.test.ts b/src/main/startup/desktop-startup-ordering.test.ts index 90cbc6dd9de..58339246287 100644 --- a/src/main/startup/desktop-startup-ordering.test.ts +++ b/src/main/startup/desktop-startup-ordering.test.ts @@ -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') }) }) diff --git a/src/main/startup/host-port-bootstrap-wiring.test.ts b/src/main/startup/host-port-bootstrap-wiring.test.ts index cfebf6cb0a8..5e6fbfdf367 100644 --- a/src/main/startup/host-port-bootstrap-wiring.test.ts +++ b/src/main/startup/host-port-bootstrap-wiring.test.ts @@ -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.