Repository navigation
fix(desktop): stop blocking startup on the login shell probe - #62667
rekram1-node wants to merge 1 commit into
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
I couldn't run the desktop app here (Electron, and the slow-shell case mostly matters on macOS), so the startup gain is unverified. Moving the probe off the main thread is the right idea, but the shell env now arrives after code that has already read process.env, and it overwrites that code's values. I reproduced the proxy part with the PR's own ShellEnv module and a fake $SHELL that sleeps 1 s and exports HTTPS_PROXY and NO_PROXY. When the proxy setup ran, HTTPS_PROXY was unset. Once the env landed, NO_PROXY had been replaced by the shell's value, dropping 127.0.0.1,localhost,::1.
Checks: bun test src/main/service passes (40 tests), and tsc -b passes. I ran tsc -b because the typecheck script needs bun check, which the local Bun 1.4.2 doesn't have.
Public API: unchanged. The change is internal to the desktop main process.
Concerns (details inline):
- Proxy setup runs too early:
loadProxyEnvironment(setGlobalProxyFromEnvand the loopbackNO_PROXYmerge) now runs before the shell env, so proxies set in.zshrcare ignored on a Dock launch, and the shell'sNO_PROXYthen overwrites the merged loopback entries. - Isolated dev mode: the late write can replace the
XDG_STATE_HOMEthatpreparesets for isolated mode.
Related: #54296
|
|
||
| if (!shell) return | ||
| const runPromise = Effect.runPromiseWith(yield* Effect.context<Path.Path>()) | ||
| ShellEnv.applyInBackground(runPromise(ShellEnv.loadShellEnv(shell)), overrides) |
There was a problem hiding this comment.
DesktopInitialization forks prepareApplicationEnvironment → loadProxyEnvironment right after this returns. That code reads the env now, before the shell env arrives: ensureLoopbackNoProxy and http.setGlobalProxyFromEnv(). On a Dock launch, HTTP(S)_PROXY from .zshrc is no longer used for main-process networking. When the shell env lands, its NO_PROXY/no_proxy overwrite the merged loopback list that the background service relies on. One way to fix it: make the proxy step wait for ShellEnv.ready, or run the loopback merge and the global proxy setup again once the env is applied.
|
|
||
| export function applyInBackground(env: Promise<Record<string, string> | null>, overrides: Record<string, string>) { | ||
| applied = env.then((value) => { | ||
| if (value) Object.assign(process.env, value, overrides) |
There was a problem hiding this comment.
This write happens at an arbitrary later time, so it overwrites anything set in process.env since startup, not just overrides. Besides NO_PROXY, this hits isolated dev mode: prepare sets XDG_STATE_HOME = userData before ensure waits for ShellEnv.ready. If the shell exports its own XDG_STATE_HOME, it replaces that value, and the spawned service gets the wrong state folder. Before this change the shell env was applied first, so later writes won. Keeping that order, for example by not overwriting keys the app has set since startup, would avoid this.
Issue for this PR
Closes #
Type of change
What does this PR do?
At startup the desktop main process reads the user's login shell environment (
$SHELL -il -c "env -0") so the background service inherits their PATH and exported variables. It usedspawnSync, so a slow.zshrc(nvm, oh-my-zsh, ...) froze the main thread for up to 5 s before IPC was registered, which kept the window on the splash screen.The probe now runs with async
spawn(same 5 s timeout, child killed on timeout) and is applied toprocess.envwhen it finishes. Only the things that start processes wait for it:BackgroundService.ensurewaits only when no compatible service is already running. Reusing a running service reads the registration file, which needs nothing from the shell (the early sidecar probe already does this before the env is loaded).whichin app detection, and "Open in" also wait for it.The result is held in memory for the process, the same approach VS Code takes (
src/vs/platform/shell/node/shellEnv.ts). Nothing is cached to disk, since the env usually contains API keys.How did you verify your code works?
Recorded production renderer builds (
electron-vite build+preview) under Xvfb, with$SHELLset to a wrapper that sleeps 3 s to stand in for a slow rc file:app starting→ service readyapp starting→ service readyAlso checked the probe directly: a fast shell resolves in ~70 ms, and a shell that hangs times out at 5 s with the event loop still running and the child killed.
bun typecheckandbun test src/main/servicepass.Visual
Screenshots / recordings
Production renderer build,
$SHELLsleeping 3 s before loading.Service already running (typical launch):
startup-warm.mp4
No service running yet (the spawn still waits for the shell env, so the gain here is only the earlier shell UI):
startup-cold.mp4
Checklist