Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

fix/shell-path-resolution: extract shared shell PATH resolver for CLI spawns - #56

Merged
shafty023 merged 3 commits into
mainfrom
fix/shell-path-resolution
Mar 27, 2026
Merged

shafty023 merged 3 commits into
mainfrom
fix/shell-path-resolution

Conversation

@shafty023

@shafty023 shafty023 commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Electron inherits a minimal PATH that excludes /opt/homebrew/bin, nvm, volta, and other user-local tool directories. CLI commands (claude, gh, codex) installed in those locations fail with ENOENT.

  • Extract getShellPath() from health-check.ts into shared shell-path.ts module
  • Replace all 14 hardcoded PATH fallbacks across 10 operation files
  • Add env to spawnStreaming calls that previously inherited bare process.env
  • Expand ~ segments before caching (spawn/execFile does not do tilde expansion)
  • Use sentinel markers to strip shell startup chatter from echo PATH output
  • Add 23 tests covering PATH resolution, tilde expansion, and output extraction

Testing: pnpm -C apps/desktop exec tsx --test test/shell-path.test.ts (23 pass) just desktop-lint, just desktop-typecheck (clean)

Risks: getShellPath spawns the login shell once on first call; 3s timeout with fallback to process.env.PATH + homebrew paths if shell hangs.

… spawns

Electron inherits a minimal PATH that excludes /opt/homebrew/bin, nvm, volta,
and other user-local tool directories. CLI commands (claude, gh, codex) installed
in those locations fail with ENOENT.

- Extract getShellPath() from health-check.ts into shared shell-path.ts module
- Replace all 14 hardcoded PATH fallbacks across 10 operation files
- Add env to spawnStreaming calls that previously inherited bare process.env
- Expand ~ segments before caching (spawn/execFile does not do tilde expansion)
- Use sentinel markers to strip shell startup chatter from echo PATH output
- Add 23 tests covering PATH resolution, tilde expansion, and output extraction

Testing: pnpm -C apps/desktop exec tsx --test test/shell-path.test.ts (23 pass)
         just desktop-lint, just desktop-typecheck (clean)

Risks: getShellPath spawns the login shell once on first call; 3s timeout
       with fallback to process.env.PATH + homebrew paths if shell hangs.
@shafty023
shafty023 requested a review from a team March 27, 2026 16:36
Tests that set process.env.PATH to a fake-bin directory need to also
reset the getShellPath() cache, otherwise the cached login-shell PATH
from a previous call takes precedence and the fake binaries are not
found (ENOENT on CI).

- Add resetShellPathCache() after every process.env.PATH assignment
  and restore in afterEach hooks across 8 test files
- Bump version to 0.9.5
@shafty023
shafty023 merged commit f3ea02c into main Mar 27, 2026
2 checks passed
@shafty023
shafty023 deleted the fix/shell-path-resolution branch March 27, 2026 17:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants