Skip to content

fix(desktop): add .zprofile wrapper to fix terminal PATH on macOS#190

Merged
saddlepaddle merged 1 commit into
mainfrom
pearl-canyon-38
Nov 30, 2025
Merged

fix(desktop): add .zprofile wrapper to fix terminal PATH on macOS#190
saddlepaddle merged 1 commit into
mainfrom
pearl-canyon-38

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented Nov 30, 2025

Summary

  • Adds .zprofile wrapper to source user's ~/.zprofile (where brew/nvm set up PATH on macOS)
  • Fixes terminals not having access to brew, npm, etc. after the shell wrapper refactor
  • Keeps ZDOTDIR pointing to wrapper dir until after .zshrc runs so superset bin gets prepended to PATH
  • Cleans up bash rcfile wrapper for consistency

Test plan

  • Lint passes
  • Typecheck passes
  • Tests pass
  • Open terminal in desktop app, verify which brew and which claude work
  • Verify claude/codex point to superset wrappers (which -a claude shows ~/.superset/bin/claude first)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Improved shell integration for zsh and bash: separates login vs interactive initialization, preserves users' existing shell configurations and nested-shell behavior, and ensures PATH modifications occur after user startup files are processed.
    • Simplified and standardized startup sourcing logic and messaging for more consistent, reliable shell startup.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
website Ready Ready Preview Comment Nov 30, 2025 2:09am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 30, 2025

Walkthrough

Reworks zsh and bash wrapper creation in the agent setup: zsh now separates login (.zprofile) and interactive (.zshrc) sourcing without changing ZDOTDIR directly; bash startup sourcing simplified to a uniform conditional sequence while preserving PATH augmentation.

Changes

Cohort / File(s) Summary
Shell wrapper refactoring
apps/desktop/src/main/lib/agent-setup.ts
Replaced the previous zsh wrapper (ZDOTDIR manipulation and single .zshrc interception) with a two-file scheme: create a .zprofile to source the user's original login files and a .zshrc that sources the user's interactive files then prepends Superset to PATH while preserving ZDOTDIR for nested shells. Simplified bash wrapper sourcing to a consistent pattern (/etc/profile, then ~/.bash_profile, ~/.bash_login, ~/.profile, and finally ~/.bashrc), kept PATH augmentation after user files, and updated comments/logging and formatting.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Verify zsh login vs interactive sourcing order and that .zprofile / .zshrc correctly source the user's originals.
  • Confirm no regressions from removing direct ZDOTDIR manipulation, including nested-shell behavior.
  • Validate PATH augmentation occurs after user startup files in both bash and zsh flows.
  • Check console messages, comment accuracy, and edge cases where user files are absent or nonstandard.

Possibly related PRs

  • superset-sh/superset PR 187 — modifies zsh/bash wrapper logic in the same agent-setup.ts area (sourcing and PATH handling).
  • superset-sh/superset PR 185 — touches shell PATH derivation/injection and related helper functions used by agent-setup.ts.

Poem

🐰 I hopped through dotfiles, soft and spry,
Split zprofile from zshrc, oh my!
Bash now greets profiles in proper time,
PATH lined up, not out of rhyme.
A tiny carrot-coded cheer, huzzah! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a .zprofile wrapper to fix terminal PATH on macOS.
Description check ✅ Passed The description covers the main changes, test plan with checkboxes, and includes the required summary section with clear context about the fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pearl-canyon-38

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b44422c and fdb36a8.

📒 Files selected for processing (1)
  • apps/desktop/src/main/lib/agent-setup.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc

Files:

  • apps/desktop/src/main/lib/agent-setup.ts
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary

Files:

  • apps/desktop/src/main/lib/agent-setup.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Avoid using any type - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code

Files:

  • apps/desktop/src/main/lib/agent-setup.ts
apps/desktop/src/main/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Node.js modules (fs, path, os, net, etc.) can be used in main process code only

Files:

  • apps/desktop/src/main/lib/agent-setup.ts
🧬 Code graph analysis (1)
apps/desktop/src/main/lib/agent-setup.ts (1)
apps/desktop/src/shared/constants.ts (1)
  • SUPERSET_DIR_NAME (21-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (4)
apps/desktop/src/main/lib/agent-setup.ts (4)

109-122: LGTM! The .zprofile wrapper correctly fixes the PATH issue.

The separation of login shell (.zprofile) and interactive shell (.zshrc) initialization is the correct approach for macOS. This ensures that Homebrew and nvm PATH configurations in the user's ~/.zprofile are properly loaded before .zshrc runs. The comment on line 116 clearly explains why ZDOTDIR is not changed at this stage.


124-133: LGTM! The ZDOTDIR reset correctly handles nested shells.

The .zshrc wrapper properly sources the user's configuration before prepending the Superset bin to PATH, ensuring wrapper scripts take precedence. The ZDOTDIR reset on line 130 is the correct approach—the first shell uses the wrapper to set PATH, while nested shells inherit PATH but use the original ZDOTDIR, avoiding redundant wrapper invocation.


140-164: LGTM! The bash wrapper follows standard initialization patterns.

The refactored bash wrapper correctly implements the standard bash startup sequence and mirrors the zsh wrapper's approach of modifying PATH after all user configurations are loaded. The cleaner comments improve readability.


208-219: LGTM! The -l flag is essential for zsh login shell initialization.

The -l flag (line 212) is critical—it ensures zsh runs in login shell mode, which sources .zprofile before .zshrc. Without it, the original ~/.zprofile (where Homebrew, nvm, and other tools set up their PATH) would never be loaded, and the wrapper's .zprofile would be skipped entirely.

The implementation correctly orchestrates:

  1. ZDOTDIR environment variable (line 198) redirects zsh to use custom wrapper files
  2. -l flag ensures .zprofile initialization runs first (sourcing original ~/.zprofile)
  3. .zshrc then sources the original ~/.zshrc and prepends Superset bin to PATH

Manual verification is warranted to confirm PATH resolution works end-to-end across the initialization sequence.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

The previous change broke terminal PATH setup because zsh wasn't
sourcing ~/.zprofile (where brew sets up PATH on macOS). This adds
a .zprofile wrapper that sources the user's real .zprofile before
.zshrc runs, ensuring brew/nvm/etc are available in terminals.

Also cleans up the bash rcfile wrapper for consistency.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@saddlepaddle saddlepaddle merged commit af7a818 into main Nov 30, 2025
7 checks passed
@Kitenite Kitenite deleted the pearl-canyon-38 branch December 1, 2025 00:16
@coderabbitai coderabbitai Bot mentioned this pull request Dec 1, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant