Skip to content

fix(desktop): source shell rc before launching CLI#332

Merged
shantur merged 5 commits into
NeuralNomadsAI:devfrom
Pagecran:fix/linux-launcher-cli-shell-dev
Apr 18, 2026
Merged

fix(desktop): source shell rc before launching CLI#332
shantur merged 5 commits into
NeuralNomadsAI:devfrom
Pagecran:fix/linux-launcher-cli-shell-dev

Conversation

@pascalandr

@pascalandr pascalandr commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Fixes #326

Summary

  • source the user's bash or zsh rc before launching the bundled CLI from Tauri
  • use -l -i -c for zsh so shell-managed Node runtimes are available in launcher-started sessions
  • fixes the reproduced Linux launcher case where the app exits with CLI exited early: exit status: 127 while terminal launches work

Validation

  • reproduced the failure with the released Tauri v0.14.0 Linux binary
  • verified the patched binary succeeds under the same launcher-like environment
  • ran cargo build on the dev-based PR branch

Linux launcher starts can miss shell-managed Node binaries even when terminal launches work. Source the user's bash/zsh rc before exec so the bundled CLI resolves the same runtime in both paths.
@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Gatekeeper review for the latest PR state.

What I understood

  • You wanted a ruthless PR gatekeeper pass on the current branch state for #332.
  • I reviewed the full PR diff against dev, checked the surrounding Tauri CLI launch code, and looked specifically for regressions and simpler/safer implementations.

Findings

  1. Blocking: zsh startup files are now executed twice
  • File: packages/tauri-app/src-tauri/src/cli_manager.rs:1300
  • File: packages/tauri-app/src-tauri/src/cli_manager.rs:1316
  • For zsh, build_shell_args() now launches zsh with -l -i -c, which already causes zsh to load its normal startup files, including ~/.zshrc for interactive shells.
  • wrap_command_for_shell() then prepends source ~/.zshrc again before the actual CLI exec.
  • That means every .zshrc side effect runs twice in launcher-started sessions. This is a real regression risk for configs that do more than export PATH: duplicate PATH mutations, repeated plugin initialization, compinit warnings/errors, duplicate prompt hooks, extra logging, and any user-defined side effects in .zshrc.
  • The safer implementation is to keep -l -i -c for zsh and remove the explicit source ~/.zshrc branch entirely. If the intent is specifically to cover bash-style setups like nvm, keep the manual rc sourcing limited to bash.

Overall

  • I did not find another stronger issue in this PR, but the zsh double-sourcing problem is enough that I would not approve this as-is.
  • After that is fixed, the approach looks directionally reasonable for the Linux launcher case described in the PR.

Suggestion

  • Minimal change: in wrap_command_for_shell(), keep the bash handling, drop the zsh handling, and let zsh -l -i -c manage its own startup sequence.

--
Yours,
CodeNomadBot

Let  manage its own startup files and keep explicit rc sourcing limited to bash, preventing duplicate zsh startup side effects in launcher-started sessions.
@pascalandr

Copy link
Copy Markdown
Contributor Author

Addressed in 0c7c11c:

  • removed the explicit source ~/.zshrc path
  • kept zsh -l -i -c so zsh manages its own startup files
  • explicit rc sourcing remains bash-only

I also rebuilt the branch after the change.

@pascalandr

Copy link
Copy Markdown
Contributor Author

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Gatekeeper review for the latest PR state.

What I understood

  • You wanted another ruthless gatekeeper pass on #332 after the follow-up fix.
  • I refreshed the PR thread, reviewed the latest diff against dev, and checked the updated shell-launch logic for regressions and better implementation options.

Findings

  • No new blocking code issues found in the latest PR state.
  • The previously reported zsh double-sourcing regression is fixed in 0c7c11c: zsh now relies on -l -i -c, while explicit rc sourcing remains bash-only.

Residual risks / validation gaps

  • This feature still intentionally depends on user shell startup behavior, so any user-owned rc output or side effects can still affect launch behavior. That is inherent to the chosen approach, not a new regression in the updated diff.
  • I attempted a local cargo build in packages/tauri-app/src-tauri, but it failed in this workspace because resources/server is missing during the Tauri build step. So I could not independently re-verify a full local build from this environment.

Overall

  • From a code review perspective, I do not see a remaining regression or a clearly better minimal implementation than the current one.
  • Conditional gatekeeper outcome: approved from diff review, with the only caveat that build validation in this environment was blocked by missing build resources rather than by the code change itself.

--
Yours,
CodeNomadBot

@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@CodeNomadBot

Re

if shell_name.contains("bash") {
        return format!(
            "if [ -f ~/.bashrc ]; then source ~/.bashrc >/dev/null 2>&1; fi; {command}"
        );
    }

Doesn't bash always load ~/.bashrc, is this needed?

  1. For current shell detection, would it be alright to use $SHELL variable?

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

What I understood

  • You are asking two review questions about the current implementation in cli_manager.rs: whether the explicit bash rc sourcing is actually needed, and whether $SHELL is an acceptable way to choose the shell.

Answers

  1. Bash does not always load ~/.bashrc here
  • In the current code, bash is invoked with -l -c, which makes it a login, non-interactive shell.
  • Bash login shells typically read ~/.bash_profile, ~/.bash_login, or ~/.profile.
  • They do not automatically read ~/.bashrc unless one of those profile files explicitly sources it.
  • So if the goal is to pick up Node managers or PATH setup that users put only in ~/.bashrc (common with nvm), the explicit source ~/.bashrc is doing real work and is not redundant.
  • By contrast, zsh with -l -i -c is interactive, so ~/.zshrc is part of zsh's normal startup path, which is why the extra manual source there was the regression.
  1. $SHELL is acceptable for this use case, with the usual caveat
  • Yes, using $SHELL is reasonable here if the intent is to select the user's preferred login shell for launching the CLI.
  • It is not a reliable way to detect the shell currently executing this exact process tree, because env vars can be stale, overridden, or inherited from another launcher.
  • But for this PR, the code is not really trying to identify the immediate parent shell; it is choosing which user shell to spawn. For that, $SHELL is a standard and pragmatic signal.
  • The existing fallback to /bin/zsh on macOS and /bin/bash elsewhere is also sensible if $SHELL is missing or empty.

Gatekeeper take

  • I do not see either of these points as a new issue in the current PR state.
  • The bash-specific rc sourcing still looks justified given the stated goal of restoring shell-managed Node runtimes in launcher-started sessions.

--
Yours,
CodeNomadBot

@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@CodeNomadBot

  1. Bash does not always load ~/.bashrc here

Do we need something similar for ~/.zshrc then?

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

What I understood

  • You are asking whether zsh also needs explicit manual ~/.zshrc sourcing, analogous to the bash-specific ~/.bashrc handling.

Answer

  • No, not with the current zsh invocation.
  • The current code runs zsh as -l -i -c.
  • The important part is -i: interactive zsh normally reads ~/.zshrc itself.
  • So adding an explicit source ~/.zshrc on top would be redundant and would reintroduce the double-sourcing regression that was already fixed.

Why bash is different here

  • Bash is currently run as -l -c, which is login but non-interactive.
  • In that mode, bash does not automatically read ~/.bashrc.
  • That is why the explicit bash-only source ~/.bashrc can still be needed.

Nuance

  • zsh login shells also read other files such as ~/.zprofile and ~/.zshenv, so zsh is already getting its normal startup sequence from the chosen flags.
  • Gatekeeper view: keeping manual rc sourcing bash-only is still the right shape for this PR.

--
Yours,
CodeNomadBot

@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@CodeNomadBot

Does bash has similar to zsh -i ?

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

What I understood

  • You are asking whether bash has an equivalent to zsh's -i interactive-shell flag.

Answer

  • Yes. Bash also supports -i for interactive mode.
  • So bash -l -i -c '...' is the rough analogue of zsh -l -i -c '...'.

Why that is not automatically the right change here

  • With bash, adding -i would make it load ~/.bashrc automatically.
  • But it would also opt into broader interactive-shell behavior, not just rc loading.
  • That can mean extra prompts, terminal-oriented setup, aliases/functions/hooks, and other user-side effects that may be less predictable in an app-launched background process.
  • The current bash approach is narrower: keep bash non-interactive and source ~/.bashrc explicitly before exec.
  • From a gatekeeper perspective, that is a more targeted way to get common PATH / Node runtime setup without fully turning the shell into an interactive session.

Bottom line

  • Yes, bash has -i.
  • But for this PR, bash -l -c plus explicit source ~/.bashrc is still the more controlled choice than switching bash to full interactive mode.

--
Yours,
CodeNomadBot

@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@pascalandr - Shall we keep it same for both zsh and bash by adding -i ?

@pascalandr

Copy link
Copy Markdown
Contributor Author

I'd keep bash as-is for now. bash -i would turn it into a fully interactive shell in a launcher/background process, which is broader and can bring extra prompt / job-control / shell-hook side effects. The current bash path is narrower: source ~/.bashrc, then exec. So I don't think bash has to match zsh 1:1 here.

@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@pascalandr

I'd keep bash as-is for now. bash -i would turn it into a fully interactive shell in a launcher/background process, which is broader and can bring extra prompt / job-control / shell-hook side effects. The current bash path is narrower: source ~/.bashrc, then exec. So I don't think bash has to match zsh 1:1 here.

  • Doesn't make sense to me, if zsh -i does same stuff and can break then why its ok to use zsh -i?
  • We have been using zsh -i / bash -i for electron till now without any issues

@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@CodeNomadBot

Verify my statement above

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

What I understood

  • You want me to verify your statement above against the codebase, specifically:
    • We have been using zsh -i / bash -i for electron till now without any issues

Verification

  • I checked packages/electron-app/electron/main/user-shell.ts.
  • The Electron implementation does support zsh with -l -i -c.
  • But bash is not using -i there. Bash currently uses -l -c.

Relevant code

  • packages/electron-app/electron/main/user-shell.ts:37
  • packages/electron-app/electron/main/user-shell.ts:39
  • packages/electron-app/electron/main/user-shell.ts:42

Specifically:

  • zsh branch: return ["-l", "-i", "-c"]
  • fallback branch, including bash: return ["-l", "-c"]

So the statement is only partially correct

  • Correct: Electron has been using zsh -i.
  • Not correct: Electron has not been using bash -i; it has been using bash without -i.
  • I also cannot verify the without any issues part from code alone. The repo shows the behavior, but not a proof that no issues ever occurred.

Gatekeeper take

  • The codebase evidence supports zsh -i precedent.
  • It does not support bash -i precedent.

--
Yours,
CodeNomadBot

@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@pascalandr - I think we can align all with zsh/bash -i -l -c to ensure we don't get weird behaviors when zsh/bash is used. It will be difficult to know.
Can you please implement this for both packages/tauri-app and packages/electron-app

Use zsh/bash -i -l -c in both desktop runtimes so interactive login shell startup stays consistent and avoids runtime differences between Electron and Tauri.
@pascalandr

Copy link
Copy Markdown
Contributor Author

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Gatekeeper review for the latest PR state.

What I understood

  • You wanted another ruthless gatekeeper pass after the new alignment commit.
  • I refreshed the PR thread, reviewed the current diff against dev, and checked both the Electron and Tauri shell-launch paths for regressions and implementation consistency.

Findings

  1. Blocking: bash can now be double-sourced in Tauri
  • File: packages/tauri-app/src-tauri/src/cli_manager.rs:1294
  • File: packages/tauri-app/src-tauri/src/cli_manager.rs:1310
  • The new Tauri code now runs bash with -i -l -c, which makes it interactive.
  • Interactive bash will read its startup files, and login bash will also read the login profile path.
  • On top of that, wrap_command_for_shell() still prepends source ~/.bashrc for bash.
  • That means the common bash setup where ~/.bash_profile or ~/.profile already sources ~/.bashrc can now execute ~/.bashrc twice in Tauri launcher sessions.
  • This is the same class of problem we already rejected for zsh earlier: duplicate PATH mutations, repeated shell hooks, repeated plugin/init code, and other user-defined side effects.
  1. The new commit does not actually align Electron and Tauri behavior
  • File: packages/electron-app/electron/main/user-shell.ts:18
  • File: packages/electron-app/electron/main/user-shell.ts:37
  • File: packages/tauri-app/src-tauri/src/cli_manager.rs:1287
  • File: packages/tauri-app/src-tauri/src/cli_manager.rs:1303
  • Electron now relies purely on -i -l -c for both bash and zsh.
  • Tauri still keeps extra bash-specific manual rc sourcing in addition to -i -l -c.
  • So the branch title/intent says "align shell invocation across Electron and Tauri", but the two implementations are still materially different in the bash case.

Overall

  • I would not approve the current state as-is.
  • The minimal safe fix is to make the Tauri bash path match Electron: if bash is now intentionally interactive, remove the manual source ~/.bashrc wrapper there too.
  • After that, I do not currently see a stronger issue in the latest diff than this one.

--
Yours,
CodeNomadBot

@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@pascalandr - Can it please be aligned.

Make Electron and Tauri launch bash and zsh the same way: login + interactive shell flags, with no extra manual rc sourcing on top.
@pascalandr

Copy link
Copy Markdown
Contributor Author

Aligned as requested in 92103bd:

  • Electron and Tauri now both use -i -l -c for bash and zsh
  • removed the extra bash-specific manual ~/.bashrc sourcing on the Tauri side, so bash no longer gets double-sourced there

I hit some workspace dependency churn while trying to rerun the local build in this worktree, but the code path is now aligned with the updated Electron behavior.

@pascalandr

Copy link
Copy Markdown
Contributor Author

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Gatekeeper review for the latest PR state.

What I understood

  • You wanted another ruthless gatekeeper pass after the latest alignment fix.
  • I refreshed the PR thread, reviewed the current diff against dev, and checked the Electron and Tauri shell-launch paths again for regressions and better implementation options.

Findings

  • No blocking regression found in the latest PR state.
  • The previous Tauri bash double-sourcing issue is fixed.
  • Electron and Tauri are now aligned on the important behavior: both use interactive login shells for bash and zsh, and neither path manually re-sources bash/zsh rc files on top of that.

Non-blocking note

  • File: packages/tauri-app/src-tauri/src/cli_manager.rs:1287
  • wrap_command_for_shell() is now effectively a pass-through helper (let _ = shell; command.to_string()).
  • That is harmless, but it is dead abstraction now. If you want the smallest cleanup, inline the command directly and remove the helper in a follow-up or before merge.

Residual validation gap

  • I did not independently re-verify a full local build from this workspace during this pass. Earlier local validation was blocked by workspace/build-resource churn rather than by a code defect, and the author noted similar churn while rechecking.

Overall

  • From a code review / gatekeeper perspective, I do not see a remaining blocker in the latest branch state.
  • I would approve this version, with only the optional cleanup note above.

--
Yours,
CodeNomadBot

Remove the now-dead wrap_command_for_shell helper after the bash/zsh alignment so the Tauri shell launch path matches the final behavior directly.
@pascalandr

Copy link
Copy Markdown
Contributor Author

wrap_command_for_shell() was suppressed and inlined in uild_shell_command_string()

@CodeNomadBot

verify my correction

@codenomadbot

codenomadbot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

What I understood

  • You want me to verify your correction that wrap_command_for_shell() was removed/suppressed and its effect inlined into build_shell_command_string().

Verification

  • I checked the current packages/tauri-app/src-tauri/src/cli_manager.rs.
  • Your correction is partially right, with one important nuance.

What is true

  • wrap_command_for_shell() is no longer present as a helper in the current file.
  • build_shell_command_string() now directly builds the command string as:
    • format!("ELECTRON_RUN_AS_NODE=1 exec {}", quoted.join(" "))
  • So yes, the final behavior is now effectively inlined there.

Nuance

  • It was not inlined by preserving the old helper call's structure.
  • The helper was removed, and build_shell_command_string() now just constructs the final command directly without any wrapper step.
  • So the precise statement is:
    • wrap_command_for_shell() was removed, and build_shell_command_string() now directly constructs the command string.

Relevant code

  • packages/tauri-app/src-tauri/src/cli_manager.rs:1241
  • packages/tauri-app/src-tauri/src/cli_manager.rs:1251
  • packages/tauri-app/src-tauri/src/cli_manager.rs:1284

Gatekeeper take

  • The correction addresses my prior non-blocking cleanup note. I no longer see that dead-helper concern in the current state.

--
Yours,
CodeNomadBot

@shantur shantur merged commit cb84547 into NeuralNomadsAI:dev Apr 18, 2026
11 of 12 checks passed
@shantur

shantur commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

Thanks @pascalandr

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.

[Bug]: Cannot launch CodeNomad through linux app laucher

2 participants