fix(electron): inherit full login shell environment instead of PATH only#62
fix(electron): inherit full login shell environment instead of PATH only#62FourWindff wants to merge 2 commits intojohannesjo:mainfrom
Conversation
…tion validation Introduced a new function to check for the existence of local branches. Enhanced the worktree creation process by validating the start-point reference and handling cases for empty repositories and non-existent branches more gracefully.
When launched from a .desktop file or AppImage, the app previously only inherited PATH from the user's login shell. Other environment variables (e.g. GOPATH, JAVA_HOME, SSH_AUTH_SOCK, NVM_DIR) were missing, causing tools that depend on them to malfunction inside spawned PTYs. Replace fixPath() with fixEnv() which runs `env -0` in an interactive login shell to capture all environment variables, then applies them to process.env via null-byte splitting. Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Updates Electron startup to inherit a complete login-shell environment (not just PATH), and adds extra git-side safeguards around main-branch detection and worktree creation.
Changes:
- Replace
fixPath()withfixEnv()to import all environment variables from an interactive login shell intoprocess.env. - Enhance default-branch detection by checking local branch refs when remote-tracking refs aren’t available.
- Add pre-validation and clearer failures when creating git worktrees from missing/invalid start refs or empty repos.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
electron/main.ts |
Captures full login-shell environment via env -0 and applies it to process.env. |
electron/ipc/git.ts |
Adds local-branch fallback for main-branch detection and validates worktree start refs with improved errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // welcome messages). Login-only (-lc) would be quieter but would miss tools | ||
| // that are only added to PATH in .bashrc/.zshrc (e.g. nvm). We accept the | ||
| // side effects since the sentinel-based parsing discards all other output. | ||
| function fixPath(): void { | ||
| function fixEnv(): void { |
There was a problem hiding this comment.
The comment above this function still describes resolving only PATH and using sentinel-based parsing to isolate PATH output, but the implementation now captures and applies the full environment via env -0. Please update the comment to match the new behavior (and any updated trade-offs).
| const result = execFileSync( | ||
| loginShell, | ||
| ['-ilc', `printf '${sentinel}' && env -0 && printf '${sentinel}'`], |
There was a problem hiding this comment.
env -0 is a GNU extension and isn’t available on macOS’s default /usr/bin/env (BSD). Since fixEnv() runs on all non-Windows platforms, this will likely throw on darwin and silently skip inheriting env/PATH. Consider either gating this logic to Linux, or adding a fallback that doesn’t require env -0 (or uses a different portable strategy on macOS).
| const result = execFileSync( | |
| loginShell, | |
| ['-ilc', `printf '${sentinel}' && env -0 && printf '${sentinel}'`], | |
| const envDumpCommand = | |
| process.platform === 'darwin' | |
| ? `/usr/bin/perl -e 'while (($k, $v) = each %ENV) { print "${k}=${v}\\0" }'` | |
| : 'env -0'; | |
| const result = execFileSync( | |
| loginShell, | |
| ['-ilc', `printf '${sentinel}' && ${envDumpCommand} && printf '${sentinel}'`], |
| const result = execFileSync( | ||
| loginShell, | ||
| ['-ilc', `printf '${sentinel}' && env -0 && printf '${sentinel}'`], | ||
| { encoding: 'utf8', timeout: 5000 }, | ||
| ); |
There was a problem hiding this comment.
execFileSync defaults to a ~1MB maxBuffer. Dumping the full login-shell environment (especially with large vars like certificates, tokens, kubeconfig, etc.) can exceed this and cause the call to throw, leaving process.env unchanged. Consider setting an explicit, larger maxBuffer (similar to MAX_BUFFER used in electron/ipc/git.ts) or handling this failure mode more explicitly.
| const isEmptyRepo = await exec('git', ['rev-list', '-n1', '--all'], { cwd: repoRoot }) | ||
| .then(({ stdout }) => !stdout.trim()) | ||
| .catch(() => true); | ||
| if (isEmptyRepo) { | ||
| throw new Error( | ||
| 'Cannot create a worktree in a repository with no commits. ' + | ||
| 'Please make an initial commit first.', | ||
| ); |
There was a problem hiding this comment.
isEmptyRepo currently treats any failure of git rev-list -n1 --all as an empty repo (catch(() => true)). If repoRoot isn’t a git repo (or git fails for another reason), this will throw the “no commits” error, which is misleading and hides the real problem. Consider distinguishing “not a git repository” (or other exec errors) from a truly empty repo, e.g. by checking isGitRepo(repoRoot) or by inspecting stderr/exit status before deciding it’s empty.
| // Validate the start-point ref exists before attempting worktree creation | ||
| const startRef = baseBranch || 'HEAD'; | ||
| try { | ||
| await exec('git', ['rev-parse', '--verify', startRef], { cwd: repoRoot }); | ||
| } catch { | ||
| const isEmptyRepo = await exec('git', ['rev-list', '-n1', '--all'], { cwd: repoRoot }) | ||
| .then(({ stdout }) => !stdout.trim()) | ||
| .catch(() => true); | ||
| if (isEmptyRepo) { | ||
| throw new Error( | ||
| 'Cannot create a worktree in a repository with no commits. ' + | ||
| 'Please make an initial commit first.', | ||
| ); | ||
| } | ||
| throw new Error( | ||
| `Branch "${startRef}" does not exist. ` + | ||
| 'Please select a valid base branch or create the branch first.', | ||
| ); | ||
| } |
There was a problem hiding this comment.
New behavior added here (validating startRef via rev-parse --verify and throwing user-facing errors for empty repos / missing base branches) isn’t covered by tests. Since electron/ipc/git.test.ts exists, it would be good to add unit tests that mock the git calls and assert the correct error is thrown for (1) empty repo and (2) nonexistent baseBranch.
| // Validate the start-point ref exists before attempting worktree creation | ||
| const startRef = baseBranch || 'HEAD'; | ||
| try { | ||
| await exec('git', ['rev-parse', '--verify', startRef], { cwd: repoRoot }); |
There was a problem hiding this comment.
The PR title/description focus on inheriting the full login-shell environment for Electron, but this file also adds new git behaviors (main branch detection changes and worktree start-ref validation with new user-facing errors). Please either update the PR description to cover these git changes (and why they’re included) or split them into a separate PR to keep scope clear.
Description
When launched from a
.desktopfile or AppImage, the app previously only inheritedPATHfrom the user's login shell viafixPath(). All other environment variables were lost, which caused two issues:GOPATH,JAVA_HOME,SSH_AUTH_SOCK, andNVM_DIRwere not available in spawned PTYs, causing tools that depend on them to fail.ANTHROPIC_BASE_URL) configured in their shell profile. Since onlyPATHwas inherited, these variables were missing and Claude could not connect at all.This PR replaces
fixPath()withfixEnv(), which runsenv -0inside an interactive login shell to capture all environment variables (null-byte separated), then applies them toprocess.env. This ensures spawned PTYs have the same complete environment as the user's terminal.Issues Resolved
.desktoplauncher only inheritedPATHfrom the login shell; all other environment variables were lost in spawned PTYs.ANTHROPIC_BASE_URL) were not passed through to the PTY.