Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughA single file modification to the zsh shell initialization wrapper script in the agent setup. The wrapper now sets ZDOTDIR for nested shells, conditionally sources multiple initialization files (.zshenv, .zshrc, .zlogin), and prepends Superset's bin directory to PATH, replacing the previous simpler approach. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/main/lib/agent-setup.ts(1 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 intsconfig.jsonwhen 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 usinganytype - 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
⏰ 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
| const zshrcScript = `# Superset zsh initialization wrapper | ||
| # This file intercepts zsh startup to ensure ~/.superset/bin is in PATH | ||
|
|
||
| # Restore original ZDOTDIR for any nested shells | ||
| export ZDOTDIR="\${SUPERSET_ORIG_ZDOTDIR:-$HOME}" | ||
|
|
||
| ORIG_ZDOTDIR="$ZDOTDIR" | ||
|
|
||
| # Source the same user-level startup files zsh normally would | ||
| if [[ -f "$ORIG_ZDOTDIR/.zshenv" ]]; then | ||
| source "$ORIG_ZDOTDIR/.zshenv" | ||
| fi | ||
|
|
||
| # Source user's real zshrc if it exists | ||
| if [[ -f "$ORIG_ZDOTDIR/.zshrc" ]]; then | ||
| source "$ORIG_ZDOTDIR/.zshrc" | ||
| fi | ||
|
|
||
| # Finish login shell sequence | ||
| if [[ -o login && -f "$ORIG_ZDOTDIR/.zlogin" ]]; then | ||
| source "$ORIG_ZDOTDIR/.zlogin" | ||
| fi | ||
|
|
||
| # Prepend superset bin to PATH (after user's rc has run) | ||
| export PATH="$HOME/${SUPERSET_DIR_NAME}/bin:$PATH" | ||
| export ZDOTDIR="$_superset_home" | ||
| `; | ||
| fs.writeFileSync(zshrcPath, zshrcScript, { mode: 0o644 }); | ||
| console.log("[agent-setup] Created zsh wrapper"); |
There was a problem hiding this comment.
Fix the zsh startup sequence violation.
The current implementation sources .zshenv and .zlogin from within .zshrc, which violates zsh's expected startup order and can cause initialization issues.
Expected zsh startup sequence: .zshenv → .zprofile → .zshrc → .zlogin
Current sequence with wrappers:
- zsh sources our
.zprofile→ sources user's.zprofile - zsh sources our
.zshrc→ sources user's.zshenv❌ (too late),.zshrc✓,.zlogin❌ (too early)
Impact:
- Environment variables in user's
.zshenvare unavailable when their.zprofileruns (breaks PATH setup dependencies) - Commands in user's
.zloginrun before.zshrccompletes (breaks timing assumptions)
Recommended fix:
Create separate wrapper files for each startup file to maintain the correct sequence:
function createZshWrapper(): void {
+ // Create .zshenv wrapper (runs first, always)
+ const zshenvPath = path.join(ZSH_DIR, ".zshenv");
+ const zshenvScript = `# Superset zsh environment wrapper
+# Restore original ZDOTDIR for nested shells
+export ZDOTDIR="\${SUPERSET_ORIG_ZDOTDIR:-$HOME}"
+
+# Source user's .zshenv if it exists
+[[ -f "$ZDOTDIR/.zshenv" ]] && source "$ZDOTDIR/.zshenv"
+`;
+ fs.writeFileSync(zshenvPath, zshenvScript, { mode: 0o644 });
+
// Create .zprofile to source user's .zprofile (runs for login shells before .zshrc)
// This is critical - without it, brew/nvm PATH setup in ~/.zprofile is skipped
const zprofilePath = path.join(ZSH_DIR, ".zprofile");
const zprofileScript = `# Superset zsh profile wrapper
-_superset_home="\${SUPERSET_ORIG_ZDOTDIR:-$HOME}"
-[[ -f "$_superset_home/.zprofile" ]] && source "$_superset_home/.zprofile"
+[[ -f "$ZDOTDIR/.zprofile" ]] && source "$ZDOTDIR/.zprofile"
`;
fs.writeFileSync(zprofilePath, zprofileScript, { mode: 0o644 });
// Create .zshrc to source user's .zshrc then prepend our bin
const zshrcPath = path.join(ZSH_DIR, ".zshrc");
const zshrcScript = `# Superset zsh initialization wrapper
-# This file intercepts zsh startup to ensure ~/.superset/bin is in PATH
-
-# Restore original ZDOTDIR for any nested shells
-export ZDOTDIR="\${SUPERSET_ORIG_ZDOTDIR:-$HOME}"
-
-ORIG_ZDOTDIR="$ZDOTDIR"
-
-# Source the same user-level startup files zsh normally would
-if [[ -f "$ORIG_ZDOTDIR/.zshenv" ]]; then
- source "$ORIG_ZDOTDIR/.zshenv"
-fi
-
# Source user's real zshrc if it exists
-if [[ -f "$ORIG_ZDOTDIR/.zshrc" ]]; then
- source "$ORIG_ZDOTDIR/.zshrc"
-fi
-
-# Finish login shell sequence
-if [[ -o login && -f "$ORIG_ZDOTDIR/.zlogin" ]]; then
- source "$ORIG_ZDOTDIR/.zlogin"
-fi
-
-# Prepend superset bin to PATH (after user's rc has run)
-export PATH="$HOME/${SUPERSET_DIR_NAME}/bin:$PATH"
+[[ -f "$ZDOTDIR/.zshrc" ]] && source "$ZDOTDIR/.zshrc"
`;
fs.writeFileSync(zshrcPath, zshrcScript, { mode: 0o644 });
+
+ // Create .zlogin wrapper (runs last for login shells)
+ const zloginPath = path.join(ZSH_DIR, ".zlogin");
+ const zloginScript = `# Superset zsh login wrapper
+# Source user's .zlogin if it exists
+[[ -f "$ZDOTDIR/.zlogin" ]] && source "$ZDOTDIR/.zlogin"
+
+# Prepend superset bin to PATH (runs at the very end)
+export PATH="$HOME/${SUPERSET_DIR_NAME}/bin:$PATH"
+`;
+ fs.writeFileSync(zloginPath, zloginScript, { mode: 0o644 });
+
console.log("[agent-setup] Created zsh wrapper");
}Note: By setting ZDOTDIR once in .zshenv and using $ZDOTDIR in subsequent wrappers, nested shells automatically use the original ZDOTDIR without needing to re-export it in every file.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/agent-setup.ts around lines 125-152, the zsh
wrapper currently sources the user's .zshenv and .zlogin from inside the .zshrc
wrapper, violating zsh's startup order; instead create separate wrapper files
that preserve the correct sequence: write a .zshenv wrapper that sets
SUPERSET_ORIG_ZDOTDIR (if not already set) and exports ZDOTDIR to point to the
wrapper directory, then write a .zprofile wrapper that sources
"$ZDOTDIR/.zprofile" if present and then runs any superset .zprofile logic,
write a .zshrc wrapper that sources "$ZDOTDIR/.zshrc" (do not source .zshenv or
.zlogin here) and finally write a .zlogin wrapper that sources
"$ZDOTDIR/.zlogin" after .zshrc; ensure each wrapper reads $ZDOTDIR (so nested
shells inherit original location), prepend the superset bin to PATH only in the
appropriate wrapper (e.g., .zprofile or .zshrc as needed), and write each file
with correct permissions and a console log for creation.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.