Repository navigation
fix(desktop): fall back to login shell probe on interactive timeout - #62736
bunnysayzz wants to merge 2 commits into
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works. With the #62732 reproduction (an interactive rc that takes longer than the 5 s probe timeout, run in a launchd-like environment), loadShellEnv returns null on the base and leaves the minimal PATH. With this PR it falls back to -l and picks up the profile's PATH. The new test fails on the base and passes here, and tsc -b for packages/desktop passes.
Public API: unchanged.
One thing to fix: the new test will fail in the Windows unit CI job (see the inline comment). Two more things, neither blocking: in the worst case, startup now waits for two probe timeouts (about 10 s) instead of one. And #62667 rewrites the same function, so whichever lands second will need a rebase.
| expect(check("/bin/zsh")).toBe(false) | ||
| }) | ||
|
|
||
| test("loadShellEnv falls back to the login shell probe when the interactive probe times out", async () => { |
There was a problem hiding this comment.
CI runs the desktop unit tests on Windows too (the unit job in check-test.yml). There, spawnSync can't run a #!/bin/sh script directly, so both probes count as unavailable, loadShellEnv returns null, and this test fails. Production never runs this probe on Windows, so test.skipIf(process.platform === "win32")(...) should be enough.
There was a problem hiding this comment.
The Windows test failure from my last review is fixed: the new test now uses test.skipIf(process.platform === "win32"). I re-ran the #62732 reproduction at this commit, and the -l fallback still loads the login PATH. bun test src/main/service/shell-env.test.ts passes 4 of 4, and tsc -b for packages/desktop passes. The public API is unchanged. I have no new concerns.
Issue for this PR
Closes #62732
Type of change
What does this PR do?
After a reboot, if the interactive shell probe (
zsh -il -c 'env -0') takes longer than 5s, Desktop gave up and left the whole session on launchd's minimal PATH, so tools like gh and node were not found until restart. The login-shell probe only ran when the interactive probe failed outright, never on a timeout. This falls through to the-lprobe on timeout too, same as the failure path. The bot repro showedbash -lalone finishes quickly, so the fallback covers the reported case.How did you verify your code works?
Added a regression test that runs loadShellEnv against a shell script that sleeps past the timeout on
-iland answers on-l. It fails on the old code (returns null) and passes with the fix. Ranbun test src/main/service/shell-env.test.tsin packages/desktop: 4 pass, 0 fail. Typecheck could not run locally, this checkout is sparse and misses the workspace packages, so CI will need to cover that.Checklist